Re: Be strict when request to flush past end of WAL in WaitXLogInsertionsToFinish

Hi Paul, Bharath,

I looked at v7. 0001 looks right to me, computing the end of the switch
record with XLogBytePosToEndRecPtr() matches what
ReserveXLogInsertLocation()
does for every other record, and what ReserveXLogSwitch() itself computes
before consuming the rest of the segment. It applies cleanly on master
(3269ccc9611) and compiles without warnings. [039_end_of_wal.pl: passes
with 0001; with 0001 reverted, fails with pg_switch_wal() = boundary + 40.]

I have a few comments:

1. This can also happen during backups, not just with pg_switch_wal().
Both do_pg_backup_start() and do_pg_backup_stop() force a WAL switch from
the
user backend, and that backend normally writes nothing else afterwards, so
XactLastRecEnd
still holds the switch record's end at commit. Those transactions have no
XID, so
RecordTransactionCommit() treats it as an async commit and passes that LSN
to XLogSetAsyncXactLSN(), the same path as
pg_switch_wal(). In other words, any backup using the low-level API can
hit the same 24-byte window, so I think we can
back-patch it and the commit message could mention this, e.g. "a backend
calling RequestXLogSwitch()" rather than only pg_switch_wal().

2. The new comment only talks about the segment boundary, but the change
also applies to a switch record ending on a page boundary mid-segment
(P + SizeOfXLogShortPHD becomes P), as Paul noted. "start of the segment"
can also be read either way. Maybe something like:

"Compute the end of the record the same way as for any other record,
so that a record ending exactly on a page boundary points to the
boundary rather than past the next page header. On a segment
boundary, the latter would be beyond the WAL we reserved."

3. As Paul earlier mentioned, CheckArchiveTimeout() says "If the returned
pointer points exactly to a segment boundary, assume nothing happened."
With 0001
that's no longer true for a switch whose record ends on the boundary. It
only skips a DEBUG1 message, but the comment (or the check) should probably
be adjusted in the same patch.

4. A minor comment, saving the reserved end and adding Assert(EndPos <=
reservedEnd) after the new computation would catch any
future regression here directly, instead of in the walwriter.

Regards,
Surya Poondla