Subject: pgsql: aio: Add test_aio module

From: Andres Freund <[email-masked]>
Date: Tue, 01 Apr 2025 18:54:58 +0000

aio: Add test_aio module

To make the tests possible, a few functions from bufmgr.c/localbuf.c had to be
exported, via buf_internals.h.

Reviewed-by: Noah Misch <[email-masked]>
Co-authored-by: Andres Freund <[email-masked]>
Co-authored-by: Nazir Bilal Yavuz <[email-masked]>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/93bc3d75d8e1aabdc256ff6da2282266dca82537

Modified Files
--------------
src/backend/storage/buffer/bufmgr.c           |    8 +-
src/backend/storage/buffer/localbuf.c         |    3 +-
src/include/storage/buf_internals.h           |    7 +
src/test/modules/Makefile                     |    1 +
src/test/modules/meson.build                  |    1 +
src/test/modules/test_aio/.gitignore          |    2 +
src/test/modules/test_aio/Makefile            |   26 +
src/test/modules/test_aio/meson.build         |   37 +
src/test/modules/test_aio/t/001_aio.pl        | 1503 +++++++++++++++++++++++++
src/test/modules/test_aio/t/002_io_workers.pl |  125 ++
src/test/modules/test_aio/test_aio--1.0.sql   |  108 ++
src/test/modules/test_aio/test_aio.c          |  806 +++++++++++++
src/test/modules/test_aio/test_aio.control    |    3 +
13 files changed, 2622 insertions(+), 8 deletions(-)

---

From: Andres Freund <[email-masked]>
Date: Tue, 1 Apr 2025 15:42:44 -0400

Hi,

I just committed the tests for AIO, and unfortunately they (so far) fail on
one buildfarm animal:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&dt=2025-04-01%2018%3A55%3A01

The reason for the failure is simple, the buildfarm animal specifies
io_method=io_uring (thanks to "cookt" for setting that up so quickly, whoever
you are :)) and the test is assuming that the -c io_method=... it passes to
initdb is actually going to be used, but it's overwritten by the TEMP_CONFIG.

I had hardened the test, with some pain, against PG_TEST_INITDB_EXTRA_OPTS
containing -c io_method=...:
	# Want to test initdb for each IO method, otherwise we could just reuse
	# the cluster.
	#
	# Unfortunately Cluster::init() puts PG_TEST_INITDB_EXTRA_OPTS after the
	# options specified by ->extra, if somebody puts -c io_method=xyz in
	# PG_TEST_INITDB_EXTRA_OPTS it would break this test. Fix that up if we
	# detect it.
	local $ENV{PG_TEST_INITDB_EXTRA_OPTS} = $ENV{PG_TEST_INITDB_EXTRA_OPTS};
	if (defined $ENV{PG_TEST_INITDB_EXTRA_OPTS}
		&& $ENV{PG_TEST_INITDB_EXTRA_OPTS} =~ m/io_method=/)
	{
		$ENV{PG_TEST_INITDB_EXTRA_OPTS} .= " -c io_method=$io_method";
	}

	$node->init(extra => [ '-c', "io_method=$io_method" ]);

But somehow I didn't think about TEMP_CONFIG.

The reason that the test passes -c io_method= to initdb is that I want to
ensure initdb passes with all the supported io_methods.  That still happens
with TEMP_CONFIG specified, it's just afterwards over-written.

I could just append io_method=$io_method again after $node->init(), but then I
couldn't verify that initdb actually ran with the to-be-tested io method.

Does anybody have a good suggestion for how to fix this?

The least bad idea I can think of is for the test to check if
PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG}) contains the string
io_method and to append the io_method again to the config if it does.  But
that seems rather ugly.

Does anybody have a better idea?

Greetings,

Andres Freund

---

From: Todd Cook <[email-masked]>
Date: Tue, 1 Apr 2025 20:12:29 +0000

On 4/1/25, 3:42 PM, "Andres Freund" <[email-masked] <mailto:[email-masked]>> wrote:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&dt=2025-04-01%2018%3A55%3A01 <https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&amp;dt=2025-04-01%2018%3A55%3A01>
>

You're welcome!

Is there an alternate way I could use to configure the io_method on bumblebee?

-- todd

---

From: Andres Freund <[email-masked]>
Date: Tue, 1 Apr 2025 16:17:05 -0400

Hi,

> Is there an alternate way I could use to configure the io_method on bumblebee?

You could use PG_TEST_INITDB_EXTRA_OPTS, but I think you did it the right
way.

For one using PG_TEST_INITDB_EXTRA_OPTS would probably require changing the
buildfarm code, because the buildfarm code filters out environment variables
that aren't on an allowlist (I really dislike that).

IOW, I think we should figure out a way to deal with TEMP_CONFIG containing
io_method=io_uring, I just don't really know how yet.

Greetings,

Andres

---

From: Noah Misch <[email-masked]>
Date: Tue, 1 Apr 2025 13:29:11 -0700

> I could just append io_method=$io_method again after $node->init(), but then I

That would be the standard way.  Here's the Cluster.pm comment that tries to
articulate the vision:

	# If a setting tends to affect whether tests pass or fail, print it after
	# TEMP_CONFIG.  Otherwise, print it before TEMP_CONFIG, thereby permitting
	# overrides.  Settings that merely improve performance or ease debugging
	# belong before TEMP_CONFIG.

Since anything initdb adds is "before TEMP_CONFIG", we have this outcome.

> Does anybody have a better idea?

Options:

1. Append, as you mention above

2. Slurp TEMP_CONFIG, as you mention above

3. Slurp postgresql.conf, and fail a test if it doesn't contain io_method.
   Then append.  If initdb fails to insert io_method, the test will catch it.

4. Run initdb outside of Cluster.pm control, then discard it

My preference order would be roughly 1,3,2,4.  The fact that initdb creates a
data directory configured for a particular io_method doesn't prove that initdb
ran with that method, so I don't see 2-4 as testing a lot beyond 1.

---

From: Andrew Dunstan <[email-masked]>
Date: Tue, 1 Apr 2025 17:08:49 -0400

> that aren't on an allowlist (I really dislike that).

Uh, not quite. Anything in the config's build_env is not filtered out. 
That change was made a year ago.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

---

From: Andres Freund <[email-masked]>
Date: Tue, 1 Apr 2025 17:34:19 -0400

Hi,

On 2025-04-01 17:08:49 -0400, Andrew Dunstan wrote:
> Uh, not quite. Anything in the config's build_env is not filtered out. That
> change was made a year ago.

Huh. Just recently I tried to configure PG_TEST_TIMEOUT_DEFAULT in build_env
and did not take effect until I explicitly added it to the @safe_set.

Greetings,

Andres Freund

---

From: Andrew Dunstan <[email-masked]>
Date: Wed, 2 Apr 2025 08:34:11 -0400

> and did not take effect until I explicitly added it to the @safe_set.

Correction: the code changed was a year ago, bit it was in release 18, 
which was in November. If it didn't work after that I want to know.

This is the commit: 
https://github.com/PGBuildFarm/client-code/commit/e2dd43915b4aa97447a5e8118f733b444896db81

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com