Subject: Re: Testing autovacuum wraparound (including failsafe)

From: Masahiko Sawada <[email-masked]>
Date: Tue, 1 Feb 2022 11:58:55 +0900

On Fri, Jun 11, 2021 at 10:19 AM Andres Freund <[email-masked]> wrote:
>
> Hi,
>
> On 2021-06-10 16:42:01 +0300, Anastasia Lubennikova wrote:
> > Cool. Thank you for working on that!
> > Could you please share a WIP patch for the $subj? I'd be happy to help with
> > it.
>
> I've attached the current WIP state, which hasn't evolved much since
> this message... I put the test in src/backend/access/heap/t/001_emergency_vacuum.pl
> but I'm not sure that's the best place. But I didn't think
> src/test/recovery is great either.
>

Thank you for sharing the WIP patch.

Regarding point (1) you mentioned (StartupSUBTRANS() takes a long time
for zeroing out all pages), how about using single-user mode instead
of preparing the transaction? That is, after pg_resetwal we check the
ages of datfrozenxid by executing a query in single-user mode. That
way, we don’t need to worry about autovacuum concurrently running
while checking the ages of frozenxids. I’ve attached a PoC patch that
does the scenario like:

1. start cluster with autovacuum=off and create tables with a few data
and make garbage on them
2. stop cluster and do pg_resetwal
3. start cluster in single-user mode
4. check age(datfrozenxid)
5. stop cluster
6. start cluster and wait for autovacuums to increase template0,
template1, and postgres datfrozenxids

I put new tests in src/test/module/heap since we already have tests
for brin in src/test/module/brin.

I think that tap test facility to run queries in single-user mode will
also be helpful for testing a new vacuum option/command that is
intended to use in emergency cases and proposed here[1].

Regards,

[1]  https://www.postgresql.org/message-id/flat/20220128012842.GZ23027%40telsasoft.com#b76c13554f90d1c8bb5532d6f3e5cbf8

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

---

From: Masahiko Sawada <[email-masked]>
Date: Thu, 30 Jun 2022 10:40:12 +0900

Hi,

> template1, and postgres datfrozenxids

The above steps are wrong.

I think we can expose a function in an extension used only by this
test in order to set nextXid to a future value with zeroing out
clog/subtrans pages. We don't need to fill all clog/subtrans pages
between oldestActiveXID and nextXid. I've attached a PoC patch for
adding this regression test and am going to register it to the next
CF.

BTW, while testing the emergency situation, I found there is a race
condition where anti-wraparound vacuum isn't invoked with the settings
autovacuum = off, autovacuum_max_workers = 1. AN autovacuum worker
sends a signal to the postmaster after advancing datfrozenxid in
SetTransactionIdLimit(). But with the settings, if the autovacuum
launcher attempts to launch a worker before the autovacuum worker who
has signaled to the postmaster finishes, the launcher exits without
launching a worker due to no free workers. The new launcher won’t be
launched until new XID is generated (and only when new XID % 65536 ==
0). Although autovacuum_max_workers = 1 is not mandatory for this
test, it's easier to verify the order of operations.

Regards,

---

From: Ian Lawrence Barwick <[email-masked]>
Date: Wed, 16 Nov 2022 13:38:10 +0900

> test, it's easier to verify the order of operations.

Hi

Thanks for the patch. While reviewing the patch backlog, we have determined that
the latest version of this patch was submitted before meson support was
implemented, so it should have a "meson.build" file added for consideration for
inclusion in PostgreSQL 16.

Regards

Ian Barwick

---

From: Heikki Linnakangas <[email-masked]>
Date: Fri, 3 Mar 2023 13:34:50 +0200

> inclusion in PostgreSQL 16.

I wanted to do some XID wraparound testing again, to test the 64-bit 
SLRUs patches [1], and revived this.

I took a different approach to consuming the XIDs. Instead of setting 
nextXID directly, bypassing GetNewTransactionId(), this patch introduces 
a helper function to call GetNewTransactionId() repeatedly. But because 
that's slow, it does include a shortcut to skip over "uninteresting" 
XIDs. Whenever nextXid is close to an SLRU page boundary or XID 
wraparound, it calls GetNewTransactionId(), and otherwise it bumps up 
nextXid close to the next "interesting" value. That's still a lot slower 
than just setting nextXid, but exercises the code more realistically.

I've written some variant of this helper function many times over the 
years, for ad hoc testing. I'd love to have it permanently in the git tree.

In addition to Masahiko's test for emergency vacuum, this includes two 
other tests. 002_limits.pl tests the "warn limit" and "stop limit" in 
GetNewTransactionId(), and 003_wraparound.pl burns through 10 billion 
transactions in total, exercising XID wraparound in general. 
Unfortunately these tests are pretty slow; the tests run for about 4 
minutes on my laptop in total, and use about 20 GB of disk space. So 
perhaps these need to be put in a special test suite that's not run as 
part of "check-world". Or perhaps leave out the 003_wraparounds.pl test, 
that's the slowest of the tests. But I'd love to have these in the git 
tree in some form.

[1] 
https://www.postgresql.org/message-id/CAJ7c6TPKf0W3MfpP2vr=[email-masked])

- Heikki

---

From: Masahiko Sawada <[email-masked]>
Date: Wed, 8 Mar 2023 13:52:31 +0900

> SLRUs patches [1], and revived this.

Thank you for reviving this thread!

> years, for ad hoc testing. I'd love to have it permanently in the git tree.

These functions seem to be better than mine.

> that's the slowest of the tests. But I'd love to have these in the git

cbfot reports some failures. The main reason seems that meson.build in
xid_wraparound directory adds the regression tests but the .sql and
.out files are missing in the patch. Perhaps the patch wants to add
only tap tests as Makefile doesn't define REGRESS?

Even after fixing this issue, CI tests (Cirrus CI) are not happy and
report failures due to a disk full. The size of xid_wraparound test
directory is 105MB out of 262MB:

% du -sh testrun
262M    testrun
% du -sh testrun/xid_wraparound/
105M    testrun/xid_wraparound/
% du -sh testrun/xid_wraparound/*
460K    testrun/xid_wraparound/001_emergency_vacuum
93M     testrun/xid_wraparound/002_limits
12M     testrun/xid_wraparound/003_wraparounds
% ls -lh testrun/xid_wraparound/002_limits/log*
total 93M
-rw-------. 1 masahiko masahiko 93M Mar  7 17:34 002_limits_wraparound.log
-rw-rw-r--. 1 masahiko masahiko 20K Mar  7 17:34 regress_log_002_limits

The biggest file is the server logs since an autovacuum worker writes
autovacuum logs for every table for every second (autovacuum_naptime
is 1s). Maybe we can set log_autovacuum_min_duration reloption for the
test tables instead of globally enabling it

The 001 test uses the 2PC transaction that holds locks on tables but
since we can consume xids while the server running, we don't need
that. Instead I think we can keep a transaction open in the background
like 002 test does.

I'll try these ideas.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

---

From: Peter Geoghegan <[email-masked]>
Date: Tue, 7 Mar 2023 21:21:00 -0800

> than just setting nextXid, but exercises the code more realistically.

Surely your tap test should be using single user mode?  Perhaps you
missed the obnoxious HINT, that's part of the WARNING that the test
parses?  ;-)

This is a very useful patch. I certainly don't want to make life
harder by (say) connecting it to the single user mode problem.
But...the single user mode thing really needs to go away. It's just
terrible advice, and actively harms users.

-- 
Peter Geoghegan

---

From: Masahiko Sawada <[email-masked]>
Date: Tue, 14 Mar 2023 15:01:30 +0900

> test tables instead of globally enabling it

I think it could be acceptable since 002 and 003 tests are executed
only when required. And 001 test seems to be able to pass on cfbot but
it takes more than 30 sec. In the attached patch, I made these tests
optional and these are enabled if envar ENABLE_XID_WRAPAROUND_TESTS is
defined (supporting only autoconf).

> that. Instead I think we can keep a transaction open in the background

Updated in the new patch. Also, I added a check if the failsafe mode
is triggered.

Regards,