Re: ACK from walreceiver to walsender

Hi Heikki,

http://git.postgresql.org/gitweb?p=users/heikki/postgres.git;a=commit;h=ebaa89ce8906e0ec45f105d083a0360b1f8bc7ca

You dropped all the ACKs from walreceiver to walsender. I have no objection
to that, but I think that walsender should handle at least 'X' (which means
that the standby is closing down the socket) and EOF (which means unexpected
loss of standby connection) messages from walreceiver. Otherwise, walsender
might be unable to detect the shutdown of walreceiver for a while.

Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:
> Hi Heikki,
>
> http://git.postgresql.org/gitweb?p=users/heikki/postgres.git;a=commit;h=ebaa89ce8906e0ec45f105d083a0360b1f8bc7ca
>
> You dropped all the ACKs from walreceiver to walsender. I have no objection
> to that, but I think that walsender should handle at least 'X' (which means
> that the standby is closing down the socket) and EOF (which means unexpected
> loss of standby connection) messages from walreceiver. Otherwise, walsender
> might be unable to detect the shutdown of walreceiver for a while.

Yeah, I just noticed that myself :-(. I guess we'll still have to use
select() in the walreceiver loop to detect that then.

I don't think we need to treat 'X' differently from EOF. You get an
error anyway if the write() fails. That's actually a bit annoying, you
get a "could not send data to client" error in the log every time a
standby disconnects for any reason.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, Jan 8, 2010 at 5:55 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> Fujii Masao wrote:
>> Hi Heikki,
>>
>> http://git.postgresql.org/gitweb?p=users/heikki/postgres.git;a=commit;h=ebaa89ce8906e0ec45f105d083a0360b1f8bc7ca
>>
>> You dropped all the ACKs from walreceiver to walsender. I have no objection
>> to that, but I think that walsender should handle at least 'X' (which means
>> that the standby is closing down the socket) and EOF (which means unexpected
>> loss of standby connection) messages from walreceiver. Otherwise, walsender
>> might be unable to detect the shutdown of walreceiver for a while.
>
> Yeah, I just noticed that myself :-(. I guess we'll still have to use
> select() in the walreceiver loop to detect that then.
>
> I don't think we need to treat 'X' differently from EOF. You get an
> error anyway if the write() fails. That's actually a bit annoying, you
> get a "could not send data to client" error in the log every time a
> standby disconnects for any reason.

Yes. And, when walreceiver exits, it sends 'X' message by calling PQfinish().
So I think it's neater for walsender to treat also 'X'. Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:
> On Fri, Jan 8, 2010 at 5:55 PM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> I don't think we need to treat 'X' differently from EOF. You get an
>> error anyway if the write() fails. That's actually a bit annoying, you
>> get a "could not send data to client" error in the log every time a
>> standby disconnects for any reason.
>
> Yes. And, when walreceiver exits, it sends 'X' message by calling PQfinish().
> So I think it's neater for walsender to treat also 'X'. Thought?

There's no guarantee walreceiver will read the 'X' before trying to
write() to the socket, so we can't rely on that to determine whether to
suppress the "could not send data to client" message.

We could try to read() from the socket after the write() has failed, to
see if there's an 'X' message pending. Not sure it's worth it. I think
we would have to put the socket into non-blocking mode before the
read(), to avoid blocking if the write() failed for some other reason.
Or select() to see if there's incoming data. I'm inclined to just not
bother..

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, Jan 8, 2010 at 6:39 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> There's no guarantee walreceiver will read the 'X' before trying to
> write() to the socket, so we can't rely on that to determine whether to
> suppress the "could not send data to client" message.

s/walreceiver/walsender?

> We could try to read() from the socket after the write() has failed, to
> see if there's an 'X' message pending. Not sure it's worth it. I think
> we would have to put the socket into non-blocking mode before the
> read(), to avoid blocking if the write() failed for some other reason.
> Or select() to see if there's incoming data. I'm inclined to just not
> bother..

Umm.. if no action is taken, walsender process would remain until
it tries to write to the socket in that case. Is this OK? I think that this
is more problematic rather than output of the "could not send data
to client" message.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:
> On Fri, Jan 8, 2010 at 6:39 PM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> There's no guarantee walreceiver will read the 'X' before trying to
>> write() to the socket, so we can't rely on that to determine whether to
>> suppress the "could not send data to client" message.
>
> s/walreceiver/walsender?

Right.

>> We could try to read() from the socket after the write() has failed, to
>> see if there's an 'X' message pending. Not sure it's worth it. I think
>> we would have to put the socket into non-blocking mode before the
>> read(), to avoid blocking if the write() failed for some other reason.
>> Or select() to see if there's incoming data. I'm inclined to just not
>> bother..
>
> Umm.. if no action is taken, walsender process would remain until
> it tries to write to the socket in that case. Is this OK?

Oh, I think we need to fix that, I'm thinking of doing a select() in the
loop to check that the socket hasn't been closed yet. I meant we don't
need to try reading the 'X' to tell apart e.g a network problem from a
standby that's shut down cleanly.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, Jan 8, 2010 at 10:35 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> Oh, I think we need to fix that, I'm thinking of doing a select() in the
> loop to check that the socket hasn't been closed yet. I meant we don't
> need to try reading the 'X' to tell apart e.g a network problem from a
> standby that's shut down cleanly.

Okey. Sorry for noise.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center