Date: Fri, 24 Sep 2021 22:21:51 -0700
From: Noah Misch <noah@leadboat.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Fixing WAL instability in various TAP tests
Message-ID: <20210925052151.GA4082552@rfd.leadboat.com>
References: <32A1FDD1-9C7B-43B1-B3EE-49198DD3F887@enterprisedb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <32A1FDD1-9C7B-43B1-B3EE-49198DD3F887@enterprisedb.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20210925052151.GA4082552%40rfd.leadboat.com>
Precedence: bulk

On Fri, Sep 24, 2021 at 05:33:13PM -0700, Mark Dilger wrote:
> A few TAP tests in the project appear to be sensitive to reductions of the
> PostgresNode's max_wal_size setting, resulting in tests failing due to wal
> files having been removed too soon.  The failures in the logs typically are
> of the "requested WAL segment %s has already been removed" variety.  I would
> expect tests which fail under legal alternate GUC settings to be hardened to
> explicitly set the GUCs as they need, rather than implicitly relying on the
> defaults.

That is not the general practice in PostgreSQL tests today.  The buildfarm
exercises some settings, so we keep the tests clean for those.  Coping with
max_wal_size=2 that way sounds reasonable.  I'm undecided about the value of
hardening tests against all possible settings.  On the plus side, it would let
us run one buildfarm member that sets every setting to its min_val or
enumvals[1] and another member that elects enumvals[cardinality(enumvals)] or
max_val.  We'd catch some real bugs that way.  On the minus side, every
nontrivial test suite addition would need to try those two cases before commit
or risk buildfarm wrath.  I don't know whether the bugs found would pay for
that trouble.  (There's also a less-important loss around the ability to
exercise a setting and manually inspect the results.  For example, I sometimes
test parallel_tuple_cost=0 parallel_setup_cost=0 and confirm a lack of
crashes.  After hardening, that would require temporary source code edits to
remove the hardening.  That's fine, though.)