[PATCH] Off-by-one error in logical slot resource retention

Hi all

I've found a minor off-by-one error in the resource retention logic
for logical slots, where we treat confirmed_flush as meaning "flushed
up to and including this LSN". Seems reasonable, but the rest of the
code treats it as "flushed up to but excluding this LSN". In
particular, we treat confirmed_flush as the inclusive startpoint for a
new logical decoding session if no startpoint is provided by the
client and will replay a change whose commit record begins at exactly
confirmed_flush to the client.

This issue was identified while debugging an issue where duplicate
rows were replicated after unclean shutdown by logical replication on
a table with no PK.

I'd prefer to make confirmed_flush mean "confirmed flushed up to and
including" everywhere, but the knock-on effects are too ugly. In
particular we'd then be changing the meaning of START_REPLICATION ...
LOGICAL ... 's argument LSN to be "start replay of commits after, but
not including, this LSN". We can't just adjust it by 1, otherwise
suddenly we'd get different results if we passed the confirmed_flush
value explicitly vs passing 0/0 and letting it be picked up
implicitly.

Two further minor patches follow, one to fix a harmless but confusing
quirk in logical walsender init, and one that adds explanatory
comments to relevant parts of the code (and a couple of spots in the
docs) to avoid future occurrences of the above issues.

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