I tried to address Andres's entirely valid complaints about that comment I
added in this patch.

I was going to add a description to restart_lsn in slot.h instead, but it's
proving harder to accurately describe restart_lsn than I'd like.

It's a point at which no transactions that commit after confirmed_lsn are
yet in progress. Easy enough.

But it's also a point at which visibility information can be reconstructed.
It's updated lazily by LogicalIncreaseRestartDecodingForSlot when the
client sends confirmation. Looking at SnapBuildProcessRunningXacts that can
be the most recent serialized snapshot from the snapshot builder (if
there's no xact running at confirmed_lsn) or the restart_decoding_lsn of
the oldest xact in progress at the confirmed_lsn. If I've read it correctly
the xact's restart_decoding_lsn is the ReorderBuffer's restart_decoding_lsn
at the time the xact started, which is in turn the last serialized snapshot
at that time.

So I'd describe restart_lsn as something like:

The oldest LSN that might be required by this replication slot.

Points to the LSN of the most recent xl_running_xacts record where no
transaction that commits after confirmed_flush is already in progress. At
this point all xacts committing after confirmed_flush can be read entirely
into reorder buffers and all visibility information can be reconstructed.

Is that accurate?

I want to get rid of the incorrect comment and fix this up.

Reply via email to