| From: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Lucas DRAESCHER <git(at)draescher(dot)fr>, Laetitia Avrot <laetitia(dot)avrot(at)gmail(dot)com>, "tanswis42(at)gmail(dot)com" <tanswis42(at)gmail(dot)com>, cca5507 <cca5507(at)qq(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [Bug Report + Patch] File descriptor leak when io_method=io_uring |
| Date: | 2026-08-11 11:31:26 |
| Message-ID: | CAKZiRmxgPg7DPmTRkgniY=ocH-iG0z5i45AYXhspRxZtdCJaTg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Jul 24, 2026 at 6:14 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Wed, Jun 17, 2026 at 6:09 PM Lucas DRAESCHER <git(at)draescher(dot)fr> wrote:
> > Thanks for the feedback, I'm happy for v2 to be the
> > design we settle on.
I believe this needs some more attention as this ring fd leak is going to
cause harder outages in larger shops (single crash is going to cause leak,
but then double allocation of the rings due to ulimit causing errno/EMFILE)
$ export PGDATA=/tmp/pg20
$ cat /tmp/pg20/postgresql.auto.conf
io_method = 'io_uring'
max_connections = 1000 # even with smaller ones we are going to hit it anyway
$ grep 2042 logfile
HINT: Consider increasing "ulimit -n" to at least 2042.
$ ulimit -n
2042
$ ls -la "/proc/$(head -1 $PGDATA/postmaster.pid)/fd/" | grep "io_uring" | wc -l
1042
# simulate single dead backend (random bug)
$ kill -9 $(psql -h /tmp -XtA postgres -c "SELECT pid FROM
pg_stat_activity WHERE backend_type = 'client backend' LIMIT 1")
$ tail -7 logfile
LOG: client backend (PID 78247) was terminated by signal 9: Killed
LOG: terminating any other active server processes
LOG: all server processes terminated; reinitializing
FATAL: could not setup io_uring queue: Too many open files
HINT: Consider increasing "ulimit -n" to at least 2042.
LOG: database system is shut down <<<<<<<!!!!!
(but it is 2042 files, but we endup with way more than 2042)
I've analyzed it also from security PoV: the leaked fds are not being visible
to children (e.g. plperlu/COPY FROM PROGRAM/etc) as they were already marked
FD_CLOEXEC internally, so that's okay from that angle.
v2 LGTM to me, but does not apply due to AioShmemInit() taking some
args now and there is this confusion (v2 preferred and less prefered v3),
so maybe v4 should be sent to avoid confusion (v2 just rebased)?
Just some nit: shouldn't we return / Assert() if not IsUnderPostmaster in
pgaio_uring_shmem_cleanup()?
> Could adding shmem_cleanup to IoMethodOps introduce an ABI break?
> If so, the v1 approach seems preferable, at least for a backpatch to v18?
I'm not sure, but it seems we add it at the end of struct (?) also it's from
aio_internal and not exposed to extensions in any way (??)
-J.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jonathan Gonzalez V. | 2026-08-11 11:32:51 | Re: [PATCH] Avoid uninitialized-value error in poll_query_until timeout diagnostic |
| Previous Message | Masashi Kamura (Fujitsu) | 2026-08-11 11:23:11 | RE: Crash issue in PG18.5 regression |