Re: 9.4 logical replication - walsender keepalive replies

Hi Steve,

On 2014-06-30 11:40:50 -0400, Steve Singer wrote:
> In 9.4 we've the below block of code to walsender.c as
>
> /*
> * 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)
> {
> WalSndKeepalive(true);
> waiting_for_ping_response = true;
> }
>
>
> I am finding that my logical replication reader is spending a tremendous
> amount of time sending feedback to the server because a keep alive reply was
> requested. My flush pointer is smaller than sendPtr, which I see as the
> normal case (The client hasn't confirmed all the wal it has been sent). My
> client queues the records it receives and only confirms when actually
> processes the record.
>
> So the sequence looks something like
>
> Server Sends LSN 0/1000
> Server Sends LSN 0/2000
> Server Sends LSN 0/3000
> Client confirms LSN 0/2000

> I don't see why all these keep alive replies are needed in this case (the
> timeout value is bumped way up, it's the above block that is triggering the
> reply request not something related to timeout)

Right. I thought about this for a while, and I think we should change
two things. For one, don't request replies here. It's simply not needed,
as this isn't dealing with timeouts. For another don't just check ->flush
< sentPtr but also && ->write < sentPtr. The reason we're sending these
feedback messages is to inform the 'logical standby' that there's been
WAL activity which it can't see because they don't correspond to
anything that's logically decoded (e.g. vacuum stuff).
Would that suit your needs?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services