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

On Sun, Mar 16, 2014 at 10:40 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> 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".

I don't think that comment needs to be updated because it means that
"until WAL < loc is flushed to the *master local* disk". It's not related to
the flush location that the standby returns. Anyway, I attached the patch.
Barring objection, I will commit this.

Regards,

--
Fujii Masao