On Fri, Jun 12, 2026 at 8:12 AM Robert Haas <[email protected]> wrote: > > Hi, > > I just tried the new GitHub Actions CI for the first time. Thanks to > everybody who worked on making that happen. However, I got a failure, > on Linux-Meson 32 bit only: > > # --- > /__w/postgresql/postgresql/contrib/postgres_fdw/expected/postgres_fdw.out > 2026-06-11 19:31:44.347832846 +0000 > # +++ > /__w/postgresql/postgresql/build/testrun/postgres_fdw-running/regress/results/postgres_fdw.out > 2026-06-11 19:47:06.867590217 +0000 > # @@ -12983,8 +12983,7 @@ > # FROM postgres_fdw_get_connections(true); > # server_name | closed | remote_backend_pid > # -------------+--------+-------------------- > # - loopback | t | t > # -(1 row) > # +(0 rows) > > Run is here: > https://github.com/robertmhaas/postgresql/actions/runs/27372091232/job/80887169394 > > Apparently, this comment isn't always correct: > > -- is not available. Despite the termination, remote_backend_pid should > -- still show the non-zero PID of the terminated remote backend. > > The issue seems to be that for the entry to appear in the output of > postgres_fdw_get_connections, it must not yet have been removed from > ConnectionHash. However, pgfdw_inval_callback can blow away > connections freely if they haven't yet been used in the current > transaction, so invalidation processing at the beginning of any > statement subsequent to the "SELECT 1 FROM ft1 LIMIT 1;" at > postgres_fdw.sql line 4607 can close the connection if a relevant > invalidation message has been received. So I guess maybe there's > enough DDL happening elsewhere concurrently with this test that a > sinval reset is possible?
I think your analysis is correct. postgres_fdw_get_connections(true) can report the connection only if the entry is still present in ConnectionHash. However, for an idle cached connection, pgfdw_inval_callback() may discard it before the next statement runs. In that case, returning 0 rows seems like a legitimate result, so I think the current comment and expected output are too strict. What seems better is to split the test expectations into two parts: 1. For the current idle-connection case, accept either outcome: if the cache entry is still present, verify that it reports closed status and a nonzero remote_backend_pid; otherwise accept 0 rows. 2. To preserve coverage of the terminated-backend reporting path, add a separate check inside an explicit transaction. In that case, the connection is already in use, so invalidation may mark it invalid but cannot discard it before transaction end, and postgres_fdw_get_connections(true) should still report it. The attached patch implements this approach. Regards, -- Fujii Masao
v1-0001-postgres_fdw-stabilize-terminated-connection-regr.patch
Description: Binary data
