Teach Page Server's WAL receiver to respond to KeepAlives

[hlinnaka]

Currently, the page server ignores PrimaryKeepAlive messages. See walreceiver.rs:

            ReplicationMessage::PrimaryKeepAlive(_keepalive) => {
                trace!("received PrimaryKeepAlive");
                // FIXME: Reply, or the connection will time out
            }

Because of that, the primary will terminate the connection after a timeout.

[ericseppanen]

This one is a bit tricky.

I think that either the keepalive response code needs to go into the replication_client code in tokio-postgres, or we need to do significant work on that code so it's possible to do send operations from the outside... (all send operations require &mut on the ReplicationClient-- which conflicts with the mutable borrow already taken to start the ReplicationStream).

@kelvich / @hlinnaka, What's the story behind that code (replication_client.rs)? Looks like it was cherry-picked from another repo? Does the code have an upstream maintainer, or should we start hacking on it any we like?

[kelvich]

What's the story behind that code (replication_client.rs)?

There is an unmerged PR with this code: sfackler/rust-postgres#696 It was not working with latest tokio, so I rebased that to a newer tokio version (as far as I remember there were no issues during rebase -- it just worked). And we are using this rebased version: https://github.com/kelvich/rust-postgres/tree/replication_rebase

I think you can move that to zenithdb account and fix it there.

At some point in time we probably should update this PR to rust-postgres.

[hlinnaka]

Note that there is a new PR in the upstream rust-postgres repository: sfackler/rust-postgres#752. I'm not sure what has changed, but we should probably update to that. The version we're using is still based on #696. (Assuming that #752 is the version that's more likely to be accepted into the official rust-postgres, I haven't followed the discussions closely.)

[ericseppanen]

(Oops, was typing this while everyone else found the same thing...)

I think I found it. There's a newer version of the code here:
sfackler/rust-postgres#752

Maybe tomorrow I'll try out that newer version and look into getting the fixes upstream.

[ericseppanen]

I think I have this mostly working.

I brought in the latest code from the tokio-postgres replication PR, which required a bunch of changes. Now I'm able to send a reply, which seems to keep the server going, even though I'm probably not sending valid data in the reply.

The reply fields are write_lsn, flush_lsn, apply_lsn, timestamp, reply. Oddly, the server seems entirely satisfied with my reply even if I set them all with 0. The connection stays up and I see incoming keepalive messages every 60 seconds.

The C code follows this rough logic:

// I think this is the last LSN seen on an incoming XLog message?
write_lsn = blockpos;
if reportFlushPosition
    flush_lsn = last_flush_position; // Is this relevant to us?
else
    flush_lsn = 0;
apply_lsn = 0; // I don't know what "apply" means here.
timestamp = feGetCurrentTimestamp(); // need to reimplement this in rust.
reply = 0; // because asking for a reply to our reply would be silly.

Does anyone know what we should put in ``write_lsn, flush_lsn, apply_lsn? If I copy the C code, at minimum I need a value for write_lsn`.

I created a new repo zenithdb/rust-postgres. Would we rather bring it in as a git submodule or refer to it by URL in Cargo.toml?

I'll post the changes as a PR, hopefully tomorrow.

[kelvich]

write_lsn, flush_lsn, apply_lsn

That ones actually are quite important. Basically it sets three lsn horizons replying to postgres up to what LSN data was written to disk, fsynced and applied to data files. And postgres may be configured for synchronous replication -- in this case it will freeze transaction commit until he receives proper acknowledgement from replication consumer (via these 3 lsn horizons). And if it that lsn is not moving forward it can stuck forever.

Also there are two slightly different scenarios when replication client sends this responses:

• whenever we received some bunch of WAL records (for example whole transaction) we may want to send this response as quickly as possible so that postgres would finish transaction commit.
• if nothing is being replicated for some amount of time postgres may ask replication consumer to respond to keepalive (that is actually case we handling here).

So write_lsn, flush_lsn, apply_lsn are probably need to go to some kind of replication context. And connect_replication() user should set these lsn to proper values, so that replication_client may answer proper values. Also method to send same response message without explicit keep-alive request would be helpful.