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

On Sun, Mar 16, 2014 at 12:24 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Fri, Mar 14, 2014 at 12:22 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Fri, Mar 14, 2014 at 3:59 AM, Fujii Masao <masao.fujii@gmail.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