Re: Wrong usage of RelationNeedsWAL

On Thu, Jan 21, 2021 at 06:02:11PM +0900, Kyotaro Horiguchi wrote:
> At Thu, 21 Jan 2021 00:19:58 -0800, Noah Misch <noah@leadboat.com> wrote in 
> > On Thu, Jan 21, 2021 at 12:28:44AM +0900, Kyotaro Horiguchi wrote:
> > > However, with the previous patch, two existing tests sto_using_cursor
> > > and sto_using_select behaves differently from the master.  That change
> > > is coming from the omission of actual LSN check in TestForOldSnapshot
> > > while wal_level=minimal. So we have no choice other than actually
> > > updating page LSN.
> > > 
> > > In the scenario under discussion there are two places we need to do
> > > that. one is heap_page_prune, and the other is RelationCopyStorge. As
> > > a PoC, I used gistXLogAssignLSN() as is for thie purpose. See the
> > > attached third file.
> > 
> > Fake LSNs make the system harder to understand, so I prefer not to spread fake
> > LSNs to more access methods.  What I had in mind is to simply suppress early
> > pruning when the relation is skipping WAL.  Attached.  Is this reasonable?  It
> > passes the older tests.  While it changes the sto_wal_optimized.spec output, I
> > think it preserves the old_snapshot_threshold behavior contract.
> 
> Perhaps I'm missing something, but the patch doesn't pass the v5-0001
> test with wal_level=minimal?

Correct.  The case we must avoid is letting an old snapshot read an
early-pruned page without error.  v5-0001 expects "ERROR:  snapshot too old".
The patch suspends early pruning, so that error is not applicable.