On 06/04/2014 01:38 AM, Josh Berkus wrote:
> On 06/03/2014 02:53 PM, Tom Lane wrote:
>> Josh Berkus <j...@agliodbs.com> writes:
>>> Out of curiosity, how much harder would it have been just to abort the
>>> transaction? I think breaking the connection is probably the right
>>> behavior, but before folks start arguing it out, I wanted to know if
>>> aborting the transaction is even a reasonable thing to do.
>> FWIW, I think aborting the transaction is probably better, especially
>> if the patch is designed to do nothing to already-aborted transactions.
>> If the client is still there, it will see the abort as a failure in its
>> next query, which is less likely to confuse it completely than a
>> connection loss. (I think, anyway.)
> Personally, I think we'll get about equal amounts of confusion either way.
>
>> The argument that we might want to close the connection to free up
>> connection slots doesn't seem to me to hold water as long as we allow
>> a client that *isn't* inside a transaction to sit on an idle connection
>> forever. Perhaps there is room for a second timeout that limits how
>> long you can sit idle independently of being in a transaction, but that
>> isn't this patch.
> Like I said, I'm marginally in favor of terminating the connection, but
> I'd be completely satisfied with a timeout which aborted the transaction
> instead. Assuming that change doesn't derail this patch and keep it
> from getting into 9.5, of course.
The change is as simple as changing the ereport from FATAL to ERROR.
Attached is a new patch doing it that way.
--
Vik
*** a/contrib/postgres_fdw/connection.c
--- b/contrib/postgres_fdw/connection.c
***************
*** 343,348 **** configure_remote_session(PGconn *conn)
--- 343,356 ----
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 abort our transaction if we keep it idle
+ * for too long.
+ * XXX: The version number must be corrected prior to commit!
+ */
+ if (remoteversion >= 90400)
+ do_sql_command(conn, "SET idle_in_transaction_timeout = 0");
}
/*
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 5545,5550 **** COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
--- 5545,5568 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-idle-in-transaction-timeout" xreflabel="idle_in_transaction_timeout">
+ <term><varname>idle_in_transaction_timeout</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>idle_in_transaction_timeout</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Abort any transaction that has been idle for longer than the specified
+ duration in seconds. This allows any locks the transaction may have taken
+ to be released.
+ </para>
+ <para>
+ A value of zero (the default) turns this off.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-vacuum-freeze-table-age" xreflabel="vacuum_freeze_table_age">
<term><varname>vacuum_freeze_table_age</varname> (<type>integer</type>)
<indexterm>
*** a/doc/src/sgml/mvcc.sgml
--- b/doc/src/sgml/mvcc.sgml
***************
*** 676,682 **** ERROR: could not serialize access due to read/write dependencies among transact
<listitem>
<para>
Don't leave connections dangling <quote>idle in transaction</quote>
! longer than necessary.
</para>
</listitem>
<listitem>
--- 676,684 ----
<listitem>
<para>
Don't leave connections dangling <quote>idle in transaction</quote>
! longer than necessary. The configuration parameter
! <xref linkend="guc-idle-in-transaction-timeout"> may be used to
! automatically abort any such transactions.
</para>
</listitem>
<listitem>
*** a/src/backend/storage/lmgr/proc.c
--- b/src/backend/storage/lmgr/proc.c
***************
*** 57,62 ****
--- 57,63 ----
int DeadlockTimeout = 1000;
int StatementTimeout = 0;
int LockTimeout = 0;
+ int IdleInTransactionTimeout = 0;
bool log_lock_waits = false;
/* Pointer to this process's PGPROC and PGXACT structs, if any */
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***************
*** 3947,3952 **** PostgresMain(int argc, char *argv[],
--- 3947,3956 ----
{
set_ps_display("idle in transaction", false);
pgstat_report_activity(STATE_IDLEINTRANSACTION, NULL);
+
+ /* Start the idle-in-transaction timer, in seconds */
+ if (IdleInTransactionTimeout > 0)
+ enable_timeout_after(IDLE_IN_TRANSACTION_TIMEOUT, 1000*IdleInTransactionTimeout);
}
else
{
***************
*** 3980,3986 **** PostgresMain(int argc, char *argv[],
DoingCommandRead = false;
/*
! * (5) check for any other interesting events that happened while we
* slept.
*/
if (got_SIGHUP)
--- 3984,3996 ----
DoingCommandRead = false;
/*
! * (5) turn off the idle-in-transaction timeout
! */
! if (IdleInTransactionTimeout > 0)
! disable_timeout(IDLE_IN_TRANSACTION_TIMEOUT, false);
!
! /*
! * (6) check for any other interesting events that happened while we
* slept.
*/
if (got_SIGHUP)
***************
*** 3990,3996 **** PostgresMain(int argc, char *argv[],
}
/*
! * (6) process the command. But ignore it if we're skipping till
* Sync.
*/
if (ignore_till_sync && firstchar != EOF)
--- 4000,4006 ----
}
/*
! * (7) process the command. But ignore it if we're skipping till
* Sync.
*/
if (ignore_till_sync && firstchar != EOF)
*** a/src/backend/utils/errcodes.txt
--- b/src/backend/utils/errcodes.txt
***************
*** 227,232 **** Section: Class 25 - Invalid Transaction State
--- 227,233 ----
25007 E ERRCODE_SCHEMA_AND_DATA_STATEMENT_MIXING_NOT_SUPPORTED schema_and_data_statement_mixing_not_supported
25P01 E ERRCODE_NO_ACTIVE_SQL_TRANSACTION no_active_sql_transaction
25P02 E ERRCODE_IN_FAILED_SQL_TRANSACTION in_failed_sql_transaction
+ 25P03 E ERRCODE_IDLE_IN_TRANSACTION_TIMEOUT idle_in_transaction_timeout
Section: Class 26 - Invalid SQL Statement Name
*** a/src/backend/utils/init/postinit.c
--- b/src/backend/utils/init/postinit.c
***************
*** 68,73 **** static void InitCommunication(void);
--- 68,74 ----
static void ShutdownPostgres(int code, Datum arg);
static void StatementTimeoutHandler(void);
static void LockTimeoutHandler(void);
+ static void IdleInTransactionTimeoutHandler(void);
static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
static void process_settings(Oid databaseid, Oid roleid);
***************
*** 550,555 **** InitPostgres(const char *in_dbname, Oid dboid, const char *username,
--- 551,557 ----
RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLock);
RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler);
RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler);
+ RegisterTimeout(IDLE_IN_TRANSACTION_TIMEOUT, IdleInTransactionTimeoutHandler);
}
/*
***************
*** 1092,1097 **** LockTimeoutHandler(void)
--- 1094,1106 ----
kill(MyProcPid, SIGINT);
}
+ static void
+ IdleInTransactionTimeoutHandler(void)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_IDLE_IN_TRANSACTION_TIMEOUT),
+ errmsg("current transaction is aborted due to idle-in-transaction timeout")));
+ }
/*
* Returns true if at least one role is defined in this database cluster.
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 1953,1958 **** static struct config_int ConfigureNamesInt[] =
--- 1953,1969 ----
},
{
+ {"idle_in_transaction_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum allowed duration of any idling transaction."),
+ gettext_noop("A value of 0 turns off the timeout."),
+ GUC_UNIT_S
+ },
+ &IdleInTransactionTimeout,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
+ {
{"vacuum_freeze_min_age", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Minimum age at which VACUUM should freeze a table row."),
NULL
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 509,514 ****
--- 509,515 ----
#session_replication_role = 'origin'
#statement_timeout = 0 # in milliseconds, 0 is disabled
#lock_timeout = 0 # in milliseconds, 0 is disabled
+ #idle_in_transaction_timeout = 0 # in seconds, 0 is disabled
#vacuum_freeze_min_age = 50000000
#vacuum_freeze_table_age = 150000000
#vacuum_multixact_freeze_min_age = 5000000
*** a/src/include/storage/proc.h
--- b/src/include/storage/proc.h
***************
*** 226,231 **** extern PGPROC *PreparedXactProcs;
--- 226,232 ----
extern int DeadlockTimeout;
extern int StatementTimeout;
extern int LockTimeout;
+ extern int IdleInTransactionTimeout;
extern bool log_lock_waits;
*** a/src/include/utils/timeout.h
--- b/src/include/utils/timeout.h
***************
*** 29,34 **** typedef enum TimeoutId
--- 29,35 ----
STATEMENT_TIMEOUT,
STANDBY_DEADLOCK_TIMEOUT,
STANDBY_TIMEOUT,
+ IDLE_IN_TRANSACTION_TIMEOUT,
/* First user-definable timeout reason */
USER_TIMEOUT,
/* Maximum number of timeout reasons */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
