Re: idle_in_transaction_timeout

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