Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. tests: Cope with io_method in TEMP_CONFIG in test_aio

  2. aio: Add test_aio module

  1. pgsql: aio: Add test_aio module

    Andres Freund <andres@anarazel.de> — 2025-04-01T18:54:58Z

    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 <noah@leadboat.com>
Co-authored-by: Andres Freund <andres@anarazel.de>
Co-authored-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
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(-)

  2. TEMP_CONFIG vs test_aio

    Andres Freund <andres@anarazel.de> — 2025-04-01T19:42:44Z

    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

  3. Re: TEMP_CONFIG vs test_aio

    Todd Cook <cookt@blackduck.com> — 2025-04-01T20:12:29Z

    On 4/1/25, 3:42 PM, "Andres Freund" <andres@anarazel.de <mailto:andres@anarazel.de>> wrote:
> 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 <https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&amp;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.

You're welcome!

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

-- todd

  4. Re: TEMP_CONFIG vs test_aio

    Andres Freund <andres@anarazel.de> — 2025-04-01T20:17:05Z

    Hi,

On 2025-04-01 20:12:29 +0000, Todd Cook wrote:
> On 4/1/25, 3:42 PM, "Andres Freund" <andres@anarazel.de <mailto:andres@anarazel.de>> wrote:
> > 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 <https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&amp;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.
>
> You're welcome!
>
> 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

  5. Re: TEMP_CONFIG vs test_aio

    Noah Misch <noah@leadboat.com> — 2025-04-01T20:29:11Z

    On Tue, Apr 01, 2025 at 03:42:44PM -0400, Andres Freund wrote:
> 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.

> 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

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.

> 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?

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.

  6. Re: TEMP_CONFIG vs test_aio

    Andrew Dunstan <andrew@dunslane.net> — 2025-04-01T21:08:49Z

    On 2025-04-01 Tu 4:17 PM, Andres Freund wrote:
> Hi,
>
> On 2025-04-01 20:12:29 +0000, Todd Cook wrote:
>> On 4/1/25, 3:42 PM, "Andres Freund" <andres@anarazel.de <mailto:andres@anarazel.de>> wrote:
>>> 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 <https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bumblebee&amp;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.
>> You're welcome!
>>
>> 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).


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

  7. Re: TEMP_CONFIG vs test_aio

    Andres Freund <andres@anarazel.de> — 2025-04-01T21:34:19Z

    Hi,

On 2025-04-01 17:08:49 -0400, Andrew Dunstan wrote:
> On 2025-04-01 Tu 4:17 PM, Andres Freund wrote:
> > 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).
> 
> 
> 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

  8. Re: TEMP_CONFIG vs test_aio

    Andrew Dunstan <andrew@dunslane.net> — 2025-04-02T12:34:11Z

    On 2025-04-01 Tu 5:34 PM, Andres Freund wrote:
> Hi,
>
> On 2025-04-01 17:08:49 -0400, Andrew Dunstan wrote:
>> On 2025-04-01 Tu 4:17 PM, Andres Freund wrote:
>>> 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).
>>
>> 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.
>


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