Re: prevent immature WAL streaming

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

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amul Sul <sulamul@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>, "Bossart, Nathan" <bossartn@amazon.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "masao.fujii@oss.nttdata.com" <masao.fujii@oss.nttdata.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "mengjuan.cmj@alibaba-inc.com" <mengjuan.cmj@alibaba-inc.com>, "Jakub.Wartak@tomtom.com" <Jakub.Wartak@tomtom.com>, Ryo Matsumura <matsumura.ryo@fujitsu.com>
Date: 2021-11-26T13:49:50Z
Lists: pgsql-hackers
On 2021-Nov-26, Amul Sul wrote:

> On Fri, Nov 26, 2021 at 1:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Meh ... but given the simplicity of the write-side fix, maybe changing
> > it is appropriate.

Actually, fixing the other side is equally simple, and it is also more
correct.  What changed my mind is that upon completing a successful read
of a record, what we set as state->ReadRecPtr is the local variable
RecPtr -- so that is what becomes the true LSN of the record.  Using
state->currRecPtr is inconsistent with that definition.

> Unless I am missing something, I am not sure why need this adjustment
> if we are going to use state->currRecPtr value which doesn't seem to
> be changing at all. AFAICU, state->currRecPtr will be unchanged value
> whether going to set overwrittenRecPtr or abortedRecPtr. Do primary
> and standby see state->currRecPtr differently, I guess not, never?

You're right for the wrong reason.  We don't need the adjustment in the
verify routine.  The reason we don't is that we're not going to use
state->currRecPtr anymore, but rather RecPtr in both places.  You're
thinking that primary and standby would never "see state->currRecPtr
differently", but that's only if they are both running the same code.
If you had a primary running 14.1 and a standby running 14.2, with the
previously proposed fix (using state->currRecPtr), you would be in
trouble.  With this fix (using RecPtr) it works fine.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  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