Обсуждение: Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

Поиск
Список
Период
Сортировка

Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Christoph Berg
Дата:
One of the AIO commits around this one (2026-03-28 00:00Z) broke the
postgresql-19 builds on apt.pg.o. Only the Debian unstable (sid,
assertions enabled) builds are working, everything else including the
nearly identical testing (forky) is broken (where assertions are off):

05:01:04 # +++ tap check in src/test/modules/test_aio +++
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, doesn't combine hits, block 0-1: expected stdout'
05:01:04 #   at t/001_aio.pl line 1474.
05:01:04 #                   '0|0|f|0
05:01:04 # 0|0|f|0'
05:01:04 #     doesn't match '(?^:^0\|0\|f\|1\n1\|1\|f\|1$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, doesn't combine hits, block 0-1: expected stderr'
05:01:04 #   at t/001_aio.pl line 1474.
05:01:04 #                   'psql:<stdin>:19: WARNING:  resource was not closed: [4728] (rel=base/5/16413, blockNum=0,
flags=0x83000000,refcount=1 1)
 
05:01:04 # psql:<stdin>:19: WARNING:  resource was not closed: [4729] (rel=base/5/16413, blockNum=1, flags=0x83000000,
refcount=11)'
 
05:01:04 #     doesn't match '(?^:^$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, interrupted by hit on 3, block 2-5: expected stdout'
05:01:04 #   at t/001_aio.pl line 1490.
05:01:04 #                   '0|2|t|1
05:01:04 # 1|3|f|0
05:01:04 # 1|3|t|2'
05:01:04 #     doesn't match '(?^:^0\|2\|t\|1\n1\|3\|f\|1\n2\|4\|t\|2$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, interrupted by hit on 3, block 2-5: expected stderr'
05:01:04 #   at t/001_aio.pl line 1490.
05:01:04 #                   'psql:<stdin>:27: WARNING:  resource was not closed: [4730] (rel=base/5/16413, blockNum=3,
flags=0x83000000,refcount=1 1)'
 
05:01:04 #     doesn't match '(?^:^$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 0: expected stdout'
05:01:04 #   at t/001_aio.pl line 1508.
05:01:04 #                   '0|0|f|0'
05:01:04 #     doesn't match '(?^:^0\|0\|f\|1$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 0: expected stderr'
05:01:04 #   at t/001_aio.pl line 1508.
05:01:04 #                   'psql:<stdin>:39: WARNING:  resource was not closed: [4734] (rel=base/5/16413, blockNum=0,
flags=0x83000000,refcount=1 1)'
 
05:01:04 #     doesn't match '(?^:^$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 1: expected stdout'
05:01:04 #   at t/001_aio.pl line 1522.
05:01:04 #                   '0|1|f|0'
05:01:04 #     doesn't match '(?^:^0\|1\|f\|1$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 1: expected stderr'
05:01:04 #   at t/001_aio.pl line 1522.
05:01:04 #                   'psql:<stdin>:47: WARNING:  resource was not closed: [4735] (rel=base/5/16413, blockNum=1,
flags=0x83000000,refcount=1 1)'
 
05:01:04 #     doesn't match '(?^:^$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 0-1: expected stdout'
05:01:04 #   at t/001_aio.pl line 1529.
05:01:04 #                   '0|0|f|0
05:01:04 # 0|0|f|0'
05:01:04 #     doesn't match '(?^:^0\|0\|f\|1\n1\|1\|f\|1$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit, block 0-1: expected stderr'
05:01:04 #   at t/001_aio.pl line 1529.
05:01:04 #                   'psql:<stdin>:51: WARNING:  resource was not closed: [4734] (rel=base/5/16413, blockNum=0,
flags=0x83000000,refcount=1 1)
 
05:01:04 # psql:<stdin>:51: WARNING:  resource was not closed: [4735] (rel=base/5/16413, blockNum=1, flags=0x83000000,
refcount=11)'
 
05:01:04 #     doesn't match '(?^:^$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit 0-1, miss 2: expected stdout'
05:01:04 #   at t/001_aio.pl line 1536.
05:01:04 #                   '0|0|f|0
05:01:04 # 0|0|f|0
05:01:04 # 0|0|t|1'
05:01:04 #     doesn't match '(?^:^0\|0\|f\|1\n1\|1\|f\|1\n2\|2\|t\|1$)'
05:01:04
05:01:04 #   Failed test 'worker: normal: read buffers, hit 0-1, miss 2: expected stderr'
05:01:04 #   at t/001_aio.pl line 1536.
05:01:04 #                   'psql:<stdin>:55: WARNING:  resource was not closed: [4734] (rel=base/5/16413, blockNum=0,
flags=0x83000000,refcount=1 1)
 
05:01:04 # psql:<stdin>:55: WARNING:  resource was not closed: [4735] (rel=base/5/16413, blockNum=1, flags=0x83000000,
refcount=11)'
 
05:01:04 #     doesn't match '(?^:^$)'
...

https://jengus.postgresql.org/view/Snapshot/job/postgresql-19-binaries-snapshot/

https://jengus.postgresql.org/view/Snapshot/job/postgresql-19-binaries-snapshot/781/architecture=amd64,distribution=forky/consoleFull

Christoph



Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Nazir Bilal Yavuz
Дата:
Hi,

On Tue, 31 Mar 2026 at 13:19, Christoph Berg <myon@debian.org> wrote:
>
> One of the AIO commits around this one (2026-03-28 00:00Z) broke the
> postgresql-19 builds on apt.pg.o. Only the Debian unstable (sid,
> assertions enabled) builds are working, everything else including the
> nearly identical testing (forky) is broken (where assertions are off):

Thank you for the report! I can reproduce this when I disable
assertions. The problem is that, in StartReadBuffersImpl() in
bufmgr.c:

```
#ifdef USE_ASSERT_CHECKING

                /*
                 * Initialize enough of ReadBuffersOperation to make
                 * CheckReadBuffersOperation() work. Outside of assertions
                 * that's not necessary when no IO is issued.
                 */
                operation->buffers = buffers;
                operation->blocknum = blockNum;
                operation->nblocks = 1;
                operation->nblocks_done = 1;
                CheckReadBuffersOperation(operation, true);
#endif
```

if (found) and if (i == 0) then we set operation->buffers and
operation->nblocks if the assertions are enabled but AIO tests expect
to read these values. So, read_buffers() in AIO tests read incorrect
values [1] when the assertions are disabled. Moving operation->buffers
and operation->nblocks outside of #ifdef USE_ASSERT_CHECKING like in
the attached fixes the problem.

[1]
read_buffers(PG_FUNCTION_ARGS)
{
    ...
    for (int nio = 0; nio < nios; nio++)
    {
        ReadBuffersOperation *operation = &operations[nio];
        int            nblocks_this_io = operation->nblocks;
        Datum        values[6] = {0};
        bool        nulls[6] = {0};
        ArrayType  *buffers_arr;

        /* convert buffer array to datum array */
        for (int i = 0; i < nblocks_this_io; i++)
        {
            Buffer        buf = operation->buffers[i];
    ...
    }


-- 
Regards,
Nazir Bilal Yavuz
Microsoft

Вложения

Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Nazir Bilal Yavuz
Дата:
Hi,

On Tue, 31 Mar 2026 at 15:10, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> Hi,
>
> On Tue, 31 Mar 2026 at 13:19, Christoph Berg <myon@debian.org> wrote:
> >
> > One of the AIO commits around this one (2026-03-28 00:00Z) broke the
> > postgresql-19 builds on apt.pg.o. Only the Debian unstable (sid,
> > assertions enabled) builds are working, everything else including the
> > nearly identical testing (forky) is broken (where assertions are off):
>
> Thank you for the report! I can reproduce this when I disable
> assertions.

Please ignore my previous email, Melanie already shared a better fix
in the original thread [1].

[1] https://postgr.es/m/CAAKRu_ZRf_t4%3D5y5nvq3BKoN8%3D8Dh77O0pi6s2kScXiAnA3eVQ%40mail.gmail.com


-- 
Regards,
Nazir Bilal Yavuz
Microsoft



Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Christoph Berg
Дата:
Re: Nazir Bilal Yavuz
> Please ignore my previous email, Melanie already shared a better fix
> in the original thread [1].

I should probably have done more research looking for an existing fix.
Admittedly I was only looking for threads with "aio" in the subject
while that one has only "IO"...

Thanks,
Christoph



Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Melanie Plageman
Дата:
On Tue, Mar 31, 2026 at 8:52 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> Please ignore my previous email, Melanie already shared a better fix
> in the original thread [1].

That's been pushed in 8519251ee975.

- Melanie



Re: pgsql: test_aio: Add basic tests for StartReadBuffers()

От
Christoph Berg
Дата:
Re: Melanie Plageman
> That's been pushed in 8519251ee975.

Confirmed, packages build again!

Thanks,
Christoph