hi, Kuroda
On 11/19/20 4:32 PM, kuroda.hay...@fujitsu.com wrote:
Dear Li,
Thanks for your suggestion. Attached!
I prefer your comments:-).
I think this patch is mostly good.
I looked whole the codes again and I found the following comment in the
PostgresMain():
```c
/*
* (5) turn off the idle-in-transaction timeout
*/
```
Please mention about idle-session timeout and check another comment.
Thanks! Add the comment for idle-session timeout.
--
Best regards
Japin Li
>From ca226701bd04d7c7f766776094610ef6a3e96279 Mon Sep 17 00:00:00 2001
From: japinli <japi...@hotmail.com>
Date: Sun, 15 Nov 2020 17:43:30 +0800
Subject: [PATCH v8 1/2] Allow terminating the idle sessions
Terminate any session that has been idle for longer than the specified
amount of time. Note that this values should be set to zero if you use
some connection-pooling software, or pg servers used by postgres_fdw,
because connections might be closed unexpectedly.
---
doc/src/sgml/config.sgml | 29 +++++++++++++++++++
src/backend/storage/lmgr/proc.c | 1 +
src/backend/tcop/postgres.c | 27 ++++++++++++++++-
src/backend/utils/errcodes.txt | 1 +
src/backend/utils/init/globals.c | 1 +
src/backend/utils/init/postinit.c | 10 +++++++
src/backend/utils/misc/guc.c | 11 +++++++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/miscadmin.h | 1 +
src/include/storage/proc.h | 1 +
src/include/utils/timeout.h | 1 +
11 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a632cf98ba..b71a116be3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8276,6 +8276,35 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
+ <varlistentry id="guc-idle-session-timeout" xreflabel="idle_session_timeout">
+ <term><varname>idle_session_timeout</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>idle_session_timeout</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Terminate any session that has been idle for longer than the specified amount of time.
+ </para>
+ <para>
+ If this value is specified without units, it is taken as milliseconds.
+ A value of zero (the default) disables the timeout.
+ </para>
+
+ <note>
+ <para>
+ This parameter should be set to zero if you use some connection-pooling software,
+ or pg servers used by postgres_fdw, because connections might be closed unexpectedly.
+ </para>
+ <para>
+ Aside from a bit of resource consumption idle sessions do not interfere with the
+ long-running stability of the server.
+ </para>
+ </note>
+
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-idle-in-transaction-session-timeout" xreflabel="idle_in_transaction_session_timeout">
<term><varname>idle_in_transaction_session_timeout</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index d1738c65f5..5fd3e79e72 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -60,6 +60,7 @@
int DeadlockTimeout = 1000;
int StatementTimeout = 0;
int LockTimeout = 0;
+int IdleSessionTimeout = 0;
int IdleInTransactionSessionTimeout = 0;
bool log_lock_waits = false;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 7c5f7c775b..ba2369b72d 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3204,6 +3204,16 @@ ProcessInterrupts(void)
}
+ if (IdleSessionTimeoutPending)
+ {
+ if (IdleSessionTimeout > 0)
+ ereport(FATAL,
+ (errcode(ERRCODE_IDLE_SESSION_TIMEOUT),
+ errmsg("terminating connection due to idle-session timeout")));
+ else
+ IdleSessionTimeoutPending = false;
+ }
+
if (ProcSignalBarrierPending)
ProcessProcSignalBarrier();
@@ -3779,6 +3789,7 @@ PostgresMain(int argc, char *argv[],
sigjmp_buf local_sigjmp_buf;
volatile bool send_ready_for_query = true;
bool disable_idle_in_transaction_timeout = false;
+ bool disable_idle_session_timeout = false;
/* Initialize startup process environment if necessary. */
if (!IsUnderPostmaster)
@@ -4227,6 +4238,14 @@ PostgresMain(int argc, char *argv[],
set_ps_display("idle");
pgstat_report_activity(STATE_IDLE, NULL);
+
+ /* Start the idle-session timer */
+ if (IdleSessionTimeout > 0)
+ {
+ disable_idle_session_timeout = true;
+ enable_timeout_after(IDLE_SESSION_TIMEOUT,
+ IdleSessionTimeout);
+ }
}
ReadyForQuery(whereToSendOutput);
@@ -4259,7 +4278,7 @@ PostgresMain(int argc, char *argv[],
DoingCommandRead = false;
/*
- * (5) turn off the idle-in-transaction timeout
+ * (5) turn off the idle-in-transaction and idle-session timeout
*/
if (disable_idle_in_transaction_timeout)
{
@@ -4267,6 +4286,12 @@ PostgresMain(int argc, char *argv[],
disable_idle_in_transaction_timeout = false;
}
+ if (disable_idle_session_timeout)
+ {
+ disable_timeout(IDLE_SESSION_TIMEOUT, false);
+ disable_idle_session_timeout = false;
+ }
+
/*
* (6) check for any other interesting events that happened while we
* slept.
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index c79312ed03..d5935a2ca9 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -109,6 +109,7 @@ Section: Class 08 - Connection Exception
08004 E ERRCODE_SQLSERVER_REJECTED_ESTABLISHMENT_OF_SQLCONNECTION sqlserver_rejected_establishment_of_sqlconnection
08007 E ERRCODE_TRANSACTION_RESOLUTION_UNKNOWN transaction_resolution_unknown
08P01 E ERRCODE_PROTOCOL_VIOLATION protocol_violation
+08008 E ERRCODE_IDLE_SESSION_TIMEOUT idle_session_timeout
Section: Class 09 - Triggered Action Exception
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 6ab8216839..03bfd88d2c 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -31,6 +31,7 @@ volatile sig_atomic_t InterruptPending = false;
volatile sig_atomic_t QueryCancelPending = false;
volatile sig_atomic_t ProcDiePending = false;
volatile sig_atomic_t ClientConnectionLost = false;
+volatile sig_atomic_t IdleSessionTimeoutPending = false;
volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false;
volatile sig_atomic_t ProcSignalBarrierPending = false;
volatile uint32 InterruptHoldoffCount = 0;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index f2dd8e4914..fbe758061b 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -71,6 +71,7 @@ static void InitCommunication(void);
static void ShutdownPostgres(int code, Datum arg);
static void StatementTimeoutHandler(void);
static void LockTimeoutHandler(void);
+static void IdleSessionTimeoutHandler(void);
static void IdleInTransactionSessionTimeoutHandler(void);
static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
@@ -628,6 +629,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLockAlert);
RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler);
RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler);
+ RegisterTimeout(IDLE_SESSION_TIMEOUT, IdleSessionTimeoutHandler);
RegisterTimeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
IdleInTransactionSessionTimeoutHandler);
}
@@ -1236,6 +1238,14 @@ LockTimeoutHandler(void)
kill(MyProcPid, SIGINT);
}
+static void
+IdleSessionTimeoutHandler(void)
+{
+ IdleSessionTimeoutPending = true;
+ InterruptPending = true;
+ SetLatch(MyLatch);
+}
+
static void
IdleInTransactionSessionTimeoutHandler(void)
{
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index bb34630e8e..e6df047b70 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2503,6 +2503,17 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"idle_session_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum allowed duration of any idling session."),
+ gettext_noop("A value of 0 turns off the timeout."),
+ GUC_UNIT_MS
+ },
+ &IdleSessionTimeout,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
{
{"idle_in_transaction_session_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the maximum allowed duration of any idling transaction."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 9cb571f7cc..6e3f14f234 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -664,6 +664,7 @@
#statement_timeout = 0 # in milliseconds, 0 is disabled
#lock_timeout = 0 # in milliseconds, 0 is disabled
#idle_in_transaction_session_timeout = 0 # in milliseconds, 0 is disabled
+#idle_session_timeout = 0 # in milliseconds, 0 is disabled
#vacuum_freeze_min_age = 50000000
#vacuum_freeze_table_age = 150000000
#vacuum_multixact_freeze_min_age = 5000000
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 72e3352398..995b603899 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -81,6 +81,7 @@
extern PGDLLIMPORT volatile sig_atomic_t InterruptPending;
extern PGDLLIMPORT volatile sig_atomic_t QueryCancelPending;
extern PGDLLIMPORT volatile sig_atomic_t ProcDiePending;
+extern PGDLLIMPORT volatile sig_atomic_t IdleSessionTimeoutPending;
extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending;
extern PGDLLIMPORT volatile sig_atomic_t ProcSignalBarrierPending;
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 00bb244340..75059539a7 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -374,6 +374,7 @@ extern PGPROC *PreparedXactProcs;
extern PGDLLIMPORT int DeadlockTimeout;
extern PGDLLIMPORT int StatementTimeout;
extern PGDLLIMPORT int LockTimeout;
+extern PGDLLIMPORT int IdleSessionTimeout;
extern PGDLLIMPORT int IdleInTransactionSessionTimeout;
extern bool log_lock_waits;
diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h
index 83a15f6795..15eeb0ed2c 100644
--- a/src/include/utils/timeout.h
+++ b/src/include/utils/timeout.h
@@ -31,6 +31,7 @@ typedef enum TimeoutId
STANDBY_TIMEOUT,
STANDBY_LOCK_TIMEOUT,
IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
+ IDLE_SESSION_TIMEOUT,
/* First user-definable timeout reason */
USER_TIMEOUT,
/* Maximum number of timeout reasons */
--
2.28.0
>From 35dcb4eab26d14d0d3897e25459fbd67826bbdea Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Sun, 30 Aug 2020 23:22:19 +1200
Subject: [PATCH v8 2/2] Optimize setitimer() usage.
Don't call setitimer() so often. Instead, let a preexisting alarm expire
and then request a new one as requried. It's better to take a few extra
spurious alarm interrupts at a rate that's a function of the timeout
setting than to have a system call rate that's a function of statement
execution frequency.
XXX: Need to review this carefully for races, perhaps involving
reentrancy.
---
src/backend/utils/misc/timeout.c | 49 ++++++++++++++++++++------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index f1c9518b0c..4effcf3be6 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -51,6 +51,13 @@ static bool all_timeouts_initialized = false;
static volatile int num_active_timeouts = 0;
static timeout_params *volatile active_timeouts[MAX_TIMEOUTS];
+/*
+ * State used to avoid installing a new timer interrupt when the previous one
+ * hasn't fired yet, but isn't too late.
+ */
+static TimestampTz sigalrm_due_at = PG_INT64_MAX;
+static volatile sig_atomic_t sigalrm_delivered = false;
+
/*
* Flag controlling whether the signal handler is allowed to do anything.
* We leave this "false" when we're not expecting interrupts, just in case.
@@ -195,12 +202,13 @@ schedule_alarm(TimestampTz now)
struct itimerval timeval;
long secs;
int usecs;
+ TimestampTz nearest_timeout;
MemSet(&timeval, 0, sizeof(struct itimerval));
/* Get the time remaining till the nearest pending timeout */
- TimestampDifference(now, active_timeouts[0]->fin_time,
- &secs, &usecs);
+ nearest_timeout = active_timeouts[0]->fin_time;
+ TimestampDifference(now, nearest_timeout, &secs, &usecs);
/*
* It's possible that the difference is less than a microsecond;
@@ -244,9 +252,18 @@ schedule_alarm(TimestampTz now)
*/
enable_alarm();
+ /*
+ * Try to avoid having to set the interval timer, if we already know
+ * that there is an undelivered signal due at the same time or sooner.
+ */
+ if (nearest_timeout >= sigalrm_due_at && !sigalrm_delivered)
+ return;
+
/* Set the alarm timer */
if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
elog(FATAL, "could not enable SIGALRM timer: %m");
+ sigalrm_due_at = nearest_timeout;
+ sigalrm_delivered = false;
}
}
@@ -266,6 +283,8 @@ handle_sig_alarm(SIGNAL_ARGS)
{
int save_errno = errno;
+ sigalrm_delivered = true;
+
/*
* Bump the holdoff counter, to make sure nothing we call will process
* interrupts directly. No timeout handler should do that, but these
@@ -423,7 +442,14 @@ reschedule_timeouts(void)
/* Reschedule the interrupt, if any timeouts remain active. */
if (num_active_timeouts > 0)
+ {
+ /*
+ * Any interruptions might be occured, so we should reschedule
+ * the active timeouts.
+ */
+ sigalrm_delivered = true;
schedule_alarm(GetCurrentTimestamp());
+ }
}
/*
@@ -591,8 +617,9 @@ disable_timeouts(const DisableTimeoutParams *timeouts, int count)
}
/*
- * Disable SIGALRM and remove all timeouts from the active list,
- * and optionally reset their timeout indicators.
+ * Remove all timeouts from the active list, and optionally reset their timeout
+ * indicators. Leave any existing itimer installed, because it may allow us to
+ * avoid having to set it again soon.
*/
void
disable_all_timeouts(bool keep_indicators)
@@ -601,20 +628,6 @@ disable_all_timeouts(bool keep_indicators)
disable_alarm();
- /*
- * Only bother to reset the timer if we think it's active. We could just
- * let the interrupt happen anyway, but it's probably a bit cheaper to do
- * setitimer() than to let the useless interrupt happen.
- */
- if (num_active_timeouts > 0)
- {
- struct itimerval timeval;
-
- MemSet(&timeval, 0, sizeof(struct itimerval));
- if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
- elog(FATAL, "could not disable SIGALRM timer: %m");
- }
-
num_active_timeouts = 0;
for (i = 0; i < MAX_TIMEOUTS; i++)
--
2.28.0
