Re: BUG #9118: WAL Sender does not disconnect replication clients during shutdown

On Sun, Mar 16, 2014 at 12:24 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Mar 14, 2014 at 12:22 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Fri, Mar 14, 2014 at 3:59 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> On second thought, I think that it's better to check a write location instead
>>> if walsender is connecting to a standby such as pg_receivexlog which
>>> always returns an invalid flush location. Attached patch does this. Thought?
>> Just a small one: could it be possible to put the condition checking
>> for the validity of flush/write position into a separate variable out
>> of this if() for readability?
>> sentPtr == MyWalSnd->flush ||
>> ! (XLogRecPtrIsInvalid(MyWalSnd->flush) &&
>> ! sentPtr == MyWalSnd->write)
>
> Yes, that's possible. What about the attached patch?
It is cleaner, thanks.

> /*
> * We only send regular messages to the client for full decoded
> * transactions, but a synchronous replication and walsender shutdown
> * possibly are waiting for a later location. So we send pings
> * containing the flush location every now and then.
> */
> if (MyWalSnd->flush < sentPtr && !waiting_for_ping_response)
> {
>
> BTW, ISTM that the above condition in walsender.c has the same problem.
> If the standby is pg_receivexlog, MyWalSnd->flush is always an invalid
> location and that condition would always be TRUE. We would need the
> same fix also there.
Comments of WalSndWaitForWal would need an update as well in this
case. It is written on top of this function that it waits "until WAL <
loc is flushed to disk". But this is not the case of pg_receivexlog as
you mentioned...
--
Michael