Abhijit Menon-Sen <ams@2ndquadrant.com>
> I've marked it Ready for Committer after a quick read-through.
I took a pass through it with an eye toward committing it. I found
a couple minor whitespace issues, where the patch didn't follow
conventional indenting practice; I can fix that no problem. I
found that as it stood, the patch reduced the number of user
timeouts which could be registered; I have a fix for that which I
hope will also prevent future problems in that regard. None of
that would have held up pushing it.
I found one substantive issue that had been missed in discussion,
though. The patch modifies the postgres_fdw extension to make it
automatically exempt from an attempt to set a limit like this on
the server to which it connects. I'm not sure that's a good idea.
Why should this type of connection be allowed to sit indefinitely
with an idle open transaction? I'm inclined to omit this part of
the patch:
+++ b/contrib/postgres_fdw/connection.c
@@ -343,6 +343,13 @@ configure_remote_session(PGconn *conn)
do_sql_command(conn, "SET extra_float_digits = 3");
else
do_sql_command(conn, "SET extra_float_digits = 2");
+
+ /*
+ * Ensure the remote server doesn't kill us off if we remain idle in a
+ * transaction for too long.
+ */
+ if (remoteversion >= 90500)
+ do_sql_command(conn, "SET idle_in_transaction_session_timeout = 0");
}
/*
(Please forgive any mangling of the whitespace above by my email
provider.)
Thoughts?
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company