Thread: TEMP_CONFIG vs test_aio

TEMP_CONFIG vs test_aio

From
Andres Freund
Date:
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



Re: TEMP_CONFIG vs test_aio

From
Todd Cook
Date:
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&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




Re: TEMP_CONFIG vs test_aio

From
Andres Freund
Date:
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&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



Re: TEMP_CONFIG vs test_aio

From
Noah Misch
Date:
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.



Re: TEMP_CONFIG vs test_aio

From
Andrew Dunstan
Date:


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

Re: TEMP_CONFIG vs test_aio

From
Andres Freund
Date:
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



Re: TEMP_CONFIG vs test_aio

From
Andrew Dunstan
Date:
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