Date: Wed, 27 Jan 2021 23:10:53 -0800
From: Noah Misch <noah@leadboat.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@postgresql.org, andres@anarazel.de
Subject: Re: Wrong usage of RelationNeedsWAL
Message-ID: <20210128071053.GB2575106@rfd.leadboat.com>
References: <20210121.180211.1817909680822521903.horikyota.ntt@gmail.com>
 <20210121092336.GB2008067@rfd.leadboat.com>
 <20210127104848.GA2508213@rfd.leadboat.com>
 <20210128.120627.428085297656501787.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210128.120627.428085297656501787.horikyota.ntt@gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Thu, Jan 28, 2021 at 12:06:27PM +0900, Kyotaro Horiguchi wrote:
> At Wed, 27 Jan 2021 02:48:48 -0800, Noah Misch <noah@leadboat.com> wrote in 
> > On Thu, Jan 21, 2021 at 01:23:36AM -0800, Noah Misch wrote:
> > > On Thu, Jan 21, 2021 at 06:02:11PM +0900, Kyotaro Horiguchi wrote:
> > > > 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.

> I studied the sto feature further and concluded that the checker side
> is fine that it always follow the chages of page-LSN.
> 
> So what we can do for the issue is setting seemingly correct page LSN
> at pruning or refrain from early-pruning while we are skipping
> WAL. The reason I took the former is I thought that the latter might
> be a problem since early-pruning would be postponed by a long-running
> wal-skipping transaction.

Yes, that's an accurate summary.

> So the patch looks fine to me. The commit message mekes sense.
> 
> However, is it ok that the existing tests (modules/snapshot_too_old)
> fails when wal_level=minimal?

That would not be okay, but I'm not seeing that.  How did your setup differ
from the following?

[nm@rfd 6:1 2021-01-27T23:06:33 postgresql 0]$ cat /nmscratch/minimal.conf
log_statement = all
wal_level = minimal
max_wal_senders = 0
log_line_prefix = '%m [%p %l %x] %q%a '
[nm@rfd 6:1 2021-01-27T23:06:38 postgresql 0]$ make -C src/test/modules/snapshot_too_old check TEMP_CONFIG=/nmscratch/minimal.conf
============== creating temporary instance            ==============
============== initializing database system           ==============
============== starting postmaster                    ==============
running on port 58080 with PID 2603099
============== creating database "isolation_regression" ==============
CREATE DATABASE
ALTER DATABASE
============== running regression test queries        ==============
test sto_using_cursor             ... ok        30168 ms
test sto_using_select             ... ok        24197 ms
test sto_using_hash_index         ... ok         6089 ms
============== shutting down postmaster               ==============
============== removing temporary instance            ==============

=====================
 All 3 tests passed.
=====================