Horton

Re: prevent immature WAL streaming

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amul Sul <sulamul@gmail.com>
Cc: "Bossart, Nathan" <bossartn@amazon.com>, "Jakub.Wartak@tomtom.com" <Jakub.Wartak@tomtom.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Ryo Matsumura <matsumura.ryo@fujitsu.com>, "andres@anarazel.de" <andres@anarazel.de>, "masao.fujii@oss.nttdata.com" <masao.fujii@oss.nttdata.com>, "mengjuan.cmj@alibaba-inc.com" <mengjuan.cmj@alibaba-inc.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-10-13T17:27:58Z
Lists: pgsql-hackers 
On 2021-Oct-13, Amul Sul wrote:

> I have one more question, regarding the need for other global
> variables i.e. abortedRecPtr.  (Sorry for coming back after so long.)
> 
> Instead of abortedRecPtr point, isn't enough to write
> overwrite-contrecord at XLogCtl->lastReplayedEndRecPtr?  I think both
> are pointing to the same location then can't we use
> lastReplayedEndRecPtr instead of abortedRecPtr to write
> overwrite-contrecord and remove need of extra global variable, like
> attached?

I'm a bit fuzzy on the difference "the end+1" and "the start of the next
record".  Are they always the same?  We do have XLogRecPtrToBytePos()
and XLogBytePosToEndRecPtr() to convert unadorned XLogRecPtr values to
"usable byte positions", which suggests to me that the proposed patch
may fail if end+1 is a page or segment boundary.

The other difference is that abortedRecPtr is set if we fail to read a
record, but XLogCtl->lastReplayedEndRecPtr is set even if we read the
record successfully.  So you'd have need a bool flag that the overwrite
contrecord record needs to be written.  Your patch is using the fact
that missingContrecPtr is non-invalid as such a flag ... I can't see
anything wrong with that.  So maybe your patch is okay in this aspect.

> You might wonder why I am so concerned about the global variable. The
> reason is that I am working on another thread[1] where we are trying
> to club all the WAL write operations that happen at the end of
> StartupXLOG into a separate function. In the future, we might want to
> allow executing this function from other processes (e.g.
> Checkpointer). For that, we need to remove the dependency of those WAL
> write operations having on the global variables which are mostly valid
> in the startup process.

Seems a fine goal.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

Commits

  1. Fix determination of broken LSN in OVERWRITTEN_CONTRECORD

  2. Fix instability in 026_overwrite_contrecord.pl test.

  3. Fix typo

  4. Change recently added test code for stability

  5. Remove unstable, unnecessary test; fix typo

  6. Repair two portability oversights of new test

  7. Fix WAL replay in presence of an incomplete record

  8. Document XLOG_INCLUDE_XID a little better

  9. Revert "Avoid creating archive status ".ready" files too early"

  10. Avoid creating archive status ".ready" files too early

  11. Fix scenario where streaming standby gets stuck at a continuation record.

  12. Teach xlogreader to follow timeline switches

  13. Forbid using pg_xlogfile_name() and pg_xlogfile_name_offset() during