CI failure: postgres_fdw_get_connections

On Fri, Jun 12, 2026 at 8:12 AM Robert Haas <robertmhaas@gmail.com> 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

On Fri, Jun 12, 2026 at 12:27 AM Fujii Masao <masao.fujii@gmail.com> wrote:

The attached patch implements this approach.

This seems reasonable to me on a quick glance. Thanks for working on it!

--
Robert Haas
EDB: http://www.enterprisedb.com

On Sun, Jun 14, 2026 at 12:18 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jun 12, 2026 at 12:27 AM Fujii Masao <masao.fujii@gmail.com> wrote:

The attached patch implements this approach.

This seems reasonable to me on a quick glance. Thanks for working on it!

Barring any objections, I will commit this patch and backpatch it to v18,
where the affected postgres_fdw_get_connections() test was added.

Regards,

--
Fujii Masao

On 15 Jul 2026, at 16:30, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sun, Jun 14, 2026 at 12:18 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jun 12, 2026 at 12:27 AM Fujii Masao <masao.fujii@gmail.com> wrote:

The attached patch implements this approach.

This seems reasonable to me on a quick glance. Thanks for working on it!

Barring any objections, I will commit this patch and backpatch it to v18,
where the affected postgres_fdw_get_connections() test was added.

LGTM. I'm not sure we need the timeout on pg_terminate_backend? That's
however not introduced with this patch, the postgres_fdw tests already use a
180s timeout, so keeping it consistent with the rest of the test code seems
correct.

--
Daniel Gustafsson

On Wed, Jul 15, 2026 at 11:50 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 15 Jul 2026, at 16:30, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sun, Jun 14, 2026 at 12:18 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jun 12, 2026 at 12:27 AM Fujii Masao <masao.fujii@gmail.com> wrote:

The attached patch implements this approach.

This seems reasonable to me on a quick glance. Thanks for working on it!

Barring any objections, I will commit this patch and backpatch it to v18,
where the affected postgres_fdw_get_connections() test was added.

LGTM. I'm not sure we need the timeout on pg_terminate_backend? That's
however not introduced with this patch, the postgres_fdw tests already use a
180s timeout, so keeping it consistent with the rest of the test code seems
correct.

Thanks for the review! I've pushed the patch.

Regards,

--
Fujii Masao