public inbox for [email protected]
help / color / mirror / Atom feedpgsql: aio: Add test_aio module
8+ messages / 4 participants
[nested] [flat]
* pgsql: aio: Add test_aio module
@ 2025-04-01 18:54 Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-04-01 18:54 UTC (permalink / raw)
To: [email protected]
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 protected]>
Co-authored-by: Andres Freund <[email protected]>
Co-authored-by: Nazir Bilal Yavuz <[email protected]>
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(-)
^ permalink raw reply [nested|flat] 8+ messages in thread
* TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
@ 2025-04-01 19:42 ` Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
2025-04-01 20:29 ` Re: TEMP_CONFIG vs test_aio Noah Misch <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Andres Freund @ 2025-04-01 19:42 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; Noah Misch <[email protected]>
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
@ 2025-04-01 20:12 ` Todd Cook <[email protected]>
2025-04-01 20:17 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Todd Cook @ 2025-04-01 20:12 UTC (permalink / raw)
To: Andres Freund <[email protected]>; [email protected] <[email protected]>; +Cc: Noah Misch <[email protected]>
On 4/1/25, 3:42 PM, "Andres Freund" <[email protected] <mailto:[email protected]>> 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&dt=2025-04-01%2018%3A55%3A...;
>
> 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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
@ 2025-04-01 20:17 ` Andres Freund <[email protected]>
2025-04-01 21:08 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-04-01 20:17 UTC (permalink / raw)
To: Todd Cook <[email protected]>; +Cc: [email protected] <[email protected]>; Noah Misch <[email protected]>
Hi,
On 2025-04-01 20:12:29 +0000, Todd Cook wrote:
> On 4/1/25, 3:42 PM, "Andres Freund" <[email protected] <mailto:[email protected]>> 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&dt=2025-04-01%2018%3A55%3A...;
> >
> > 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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
2025-04-01 20:17 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
@ 2025-04-01 21:08 ` Andrew Dunstan <[email protected]>
2025-04-01 21:34 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Dunstan @ 2025-04-01 21:08 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Todd Cook <[email protected]>; +Cc: [email protected] <[email protected]>; Noah Misch <[email protected]>
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" <[email protected] <mailto:[email protected]>> 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&dt=2025-04-01%2018%3A55%3A...;
>>>
>>> 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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
2025-04-01 20:17 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 21:08 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
@ 2025-04-01 21:34 ` Andres Freund <[email protected]>
2025-04-02 12:34 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-04-01 21:34 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Todd Cook <[email protected]>; [email protected] <[email protected]>; Noah Misch <[email protected]>
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
2025-04-01 20:17 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 21:08 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
2025-04-01 21:34 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
@ 2025-04-02 12:34 ` Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Dunstan @ 2025-04-02 12:34 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Todd Cook <[email protected]>; [email protected] <[email protected]>; Noah Misch <[email protected]>
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: TEMP_CONFIG vs test_aio
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
@ 2025-04-01 20:29 ` Noah Misch <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Noah Misch @ 2025-04-01 20:29 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]
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.
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2025-04-02 12:34 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-04-01 18:54 pgsql: aio: Add test_aio module Andres Freund <[email protected]>
2025-04-01 19:42 ` TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 20:12 ` Re: TEMP_CONFIG vs test_aio Todd Cook <[email protected]>
2025-04-01 20:17 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-01 21:08 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
2025-04-01 21:34 ` Re: TEMP_CONFIG vs test_aio Andres Freund <[email protected]>
2025-04-02 12:34 ` Re: TEMP_CONFIG vs test_aio Andrew Dunstan <[email protected]>
2025-04-01 20:29 ` Re: TEMP_CONFIG vs test_aio Noah Misch <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox