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

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?

> Are there risks that a standby sends an invalid flush location and a
> valid write location?

No unless I'm missing something.

/*
* 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.

Regards,

--
Fujii Masao