Date: Fri, 26 Nov 2021 10:49:50 -0300
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>
Subject: Re: prevent immature WAL streaming
Message-ID: <202111261349.2vo5zmicesqp@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAAJ_b97Jwd=btx95mF9e8zaeWcwpbJ257S5y5n2j4iRniuMogg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202111261349.2vo5zmicesqp%40alvherre.pgsql>
Precedence: bulk

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/