The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Hi,

I looked at v3, focusing on the reported io_uring file descriptor leak across
backend crash restarts.

I used the v3 attachment from Lucas's 2026-04-17 message, on origin/master at
4cb2a9863d89b320f37eb1bd76822f6f65e59311.

The patch applies cleanly with git am, and git diff --check reports no issues.

I first did a non-liburing build on macOS:

./configure --prefix=.../pg-install --without-readline --without-zlib 
--without-icu
make -s -j8
make -s install

That passed. I could not run TAP tests locally there because IPC::Run is not
installed, and that build did not have liburing support.

I then tested on Linux 6.8.0 with liburing 2.5.  I configured with:

./configure --prefix="$PWD/pg-install" --without-readline --without-zlib 
--without-icu --enable-tap-tests --with-liburing
make -s -j3
make -s install

configure found liburing and io_uring_queue_init_mem, and the build/install
passed.

The existing AIO TAP tests passed:

make -C src/test/modules/test_aio check

Result:

All tests successful.
Files=4, Tests=780
Result: PASS

For the reported leak, I compared unpatched master and v3 using an isolated
server configured with io_method=io_uring.  In each cycle the script counted
the postmaster's /proc/<pid>/fd symlinks containing io_uring, started a client
backend running pg_sleep(), killed that backend with SIGKILL, waited for the
postmaster to reinitialize after the crash restart, and counted again.

On unpatched master:

initial_io_uring_fds=52
cycle=1 before=52 after=104 delta=52
cycle=2 before=104 after=156 delta=52
cycle=3 before=156 after=208 delta=52

With v3:

initial_io_uring_fds=52
cycle=1 before=52 after=52 delta=0
cycle=2 before=52 after=52 delta=0
cycle=3 before=52 after=52 delta=0

The v3 server log also showed the cleanup running at each reinitialization:

DEBUG:  cleaning up 52 io_uring processes

So the reported leak reproduces for me on master, and v3 fixes it in this
reproducer.

I did not find a new issue in the checked path.

I have not reviewed older stable branches or the backpatching question, and I
did not run installcheck-world.

Regards,
Ilmar Yunusov

The new status of this patch is: Ready for Committer

Reply via email to