| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Noah Misch <noah(at)leadboat(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
| Subject: | [PATCH] Small gaps in IoWorkerMain() error handling |
| Date: | 2026-10-16 12:24:00 |
| Message-ID: | CAJTYsWVQ=WS+iCaKtWpB_Yv7wnDRF-SyLwCN3XrJP08mH=KJWQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While working on an unrelated fsync patch I got this assertion failure
from an I/O worker:
TRAP: failed Assert("error_errno != 0"), File: "method_worker.c", Line: 747
I was injecting errors at the time, with a log hook that throws an ERROR
from the worker's DEBUG4 "processing IO" message, so it's probably not
something anyone hits in practice. Still, it looks like a real gap:
error_ioh is already set when that message is emitted, but error_errno
isn't set until just before pgaio_io_reopen(), so the error handler is
left with error_errno == 0.
The attached 0001 just moves the ENOENT assignment up so it happens
before error_ioh is set. I'm not sure ENOENT is the best errno when the
failure has nothing to do with the file, but it's what the reopen
failure path already uses, so I left it alone for now.
While looking at this I noticed a second problem in the same spot.
errfinish() does a CHECK_FOR_INTERRUPTS() after emitting the message,
and the worker hasn't done HOLD_INTERRUPTS() yet at that point. If a
SIGINT is pending then, the worker exits with the IO dequeued but never
completed, and anything waiting on it just hangs. (I got that by sending
SIGINT from the same hook.)
0002 moves the debug message below HOLD_INTERRUPTS(). As far as I can
tell that message is the only place interrupts get processed in that
window, so this seemed like the least invasive fix. Another option would
be to fail whatever IO the worker owns from an exit callback, which
would also cover anything added there later, but that felt like
overkill to me esp. considering the backpatching scenario...
Regards,
Ayush
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-aio-Set-error_errno-before-error_ioh-in-I-O-workers.patch | application/octet-stream | 1.8 KB |
| v1-0002-aio-Hold-interrupts-earlier-in-I-O-workers.patch | application/octet-stream | 1.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Maxim Orlov | 2026-10-16 12:36:29 | Re: Patch for migration of the pg_commit_ts directory |
| Previous Message | Jim Jones | 2026-10-16 12:18:38 | Re: PoC: Simplify recovery after dropping a table by LOGGING the restore LSN |