From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
Cc: | Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Potential deadlock in pgaio_io_wait() |
Date: | 2025-08-04 05:54:45 |
Message-ID: | CA+hUKG+mZYrSdnhk-XrBYO18H829K77S9gMKUsykOiTJtqB43g@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
Hi,
I think this sequence can deadlock if there is a ->wait_one() function
needed for progress:
P1 P2
acquire lock
stage io
wait for io cv...
submit io
acquire lock...
It's OK to assume that the state will advance to SUBMITTED, but not
any further as the current coding does. And even without a deadlock,
P2 might just take a long time to get around to completing, when P1
should ideally handle it.
A naive solution would be to broadcast on ioh->cv after
pgaio_io_prepare_submit()'s state change, with a case to wait for
that. But that's expensive.
It might be slightly better to have a dedicated per-backend submit_cv,
so the owner only has to broadcast once per batch. If ->submit() does
significant per-IO work then maybe that's a bit later than it needs to
be, but it's really just one system call anyway, so I doubt it
matters. Sketch attached for illustration only.
A real improvement would be to avoid the broadcast when there are no
waiters, but I got a bit too confused about flags and memory barriers
so I'm just sharing this problem report and illustration-grade patch
for now.
I doubt it's very easy to reproduce with simple queries, but I assume
if you had a SQL function that acquires a central LWLock and you ran
concurrent queries SELECT COUNT(*) FROM t WHERE locking_function(x) it
in a tight loop in two sessions given t big enough to require a
BufferAccessStrategy so that we keep evicting the buffers and
streaming them back in, you must be able to reach it. It might be
easier to write test functions that submit and wait for IO handles
directly across two sessions or something.
Attachment | Content-Type | Size |
---|---|---|
0001-aio-Fix-pgaio_io_wait-for-DEFINED-and-STAGED.patch | application/octet-stream | 4.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2025-08-04 06:34:41 | Re: Potential deadlock in pgaio_io_wait() |
Previous Message | David Rowley | 2025-08-04 04:28:33 | Re: BUG #19007: Planner fails to choose partial index with spurious 'not null' |
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2025-08-04 05:55:08 | Re: implement CAST(expr AS type FORMAT 'template') |
Previous Message | David Rowley | 2025-08-04 05:44:13 | Re: Fix a typo of mod_since_analyze in do_analyze_rel |