Autovacuum and idle_session_timeout
Hello,
I've been reading the autovacuum code (the launcher and the worker) on the
14 branch. As previously, I've seen some configuration at the beginning,
especially for statement_timeout, lock_timeout and
idle_in_transaction_session_timeout, and I was surprised to discover there
was no configuration for idle_session_timeout. I'm not sure the code should
set it to 0 as well (otherwise I'd have written a patch), but, if there was
a decision made to ignore its value, I'd be interested to know the reason.
I could guess for the autovacuum worker (it seems to work in a transaction,
so it's already handled by the idle_in_transaction_timeout), but I have no
idea for the autovacuum launcher.
If it was just missed, I could write a patch this week to fix this.
Regards.
--
Guillaume.
On Thu, 30 Dec 2021 at 17:18, Guillaume Lelarge <guillaume@lelarge.info> wrote:
Hello,
I've been reading the autovacuum code (the launcher and the worker) on the
14 branch. As previously, I've seen some configuration at the beginning,
especially for statement_timeout, lock_timeout and
idle_in_transaction_session_timeout, and I was surprised to discover there
was no configuration for idle_session_timeout. I'm not sure the code should
set it to 0 as well (otherwise I'd have written a patch), but, if there was
a decision made to ignore its value, I'd be interested to know the reason.
I could guess for the autovacuum worker (it seems to work in a transaction,
so it's already handled by the idle_in_transaction_timeout), but I have no
idea for the autovacuum launcher.If it was just missed, I could write a patch this week to fix this.
Oh, it was just missed. I didn't note set autovacuum code set those settings,
I think we should also set idle_session_timeout to 0.
Should we also change this for pg_dump and pg_backup_archiver?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Le jeu. 30 déc. 2021 à 11:44, Japin Li <japinli@hotmail.com> a écrit :
On Thu, 30 Dec 2021 at 17:18, Guillaume Lelarge <guillaume@lelarge.info>
wrote:Hello,
I've been reading the autovacuum code (the launcher and the worker) on
the
14 branch. As previously, I've seen some configuration at the beginning,
especially for statement_timeout, lock_timeout and
idle_in_transaction_session_timeout, and I was surprised to discoverthere
was no configuration for idle_session_timeout. I'm not sure the code
should
set it to 0 as well (otherwise I'd have written a patch), but, if there
was
a decision made to ignore its value, I'd be interested to know the
reason.
I could guess for the autovacuum worker (it seems to work in a
transaction,
so it's already handled by the idle_in_transaction_timeout), but I have
no
idea for the autovacuum launcher.
If it was just missed, I could write a patch this week to fix this.
Oh, it was just missed. I didn't note set autovacuum code set those
settings,
I think we should also set idle_session_timeout to 0.Should we also change this for pg_dump and pg_backup_archiver?
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.
--
Guillaume.
On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge <guillaume@lelarge.info> wrote:
Le jeu. 30 déc. 2021 à 11:44, Japin Li <japinli@hotmail.com> a écrit :
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.
Attached fix this, please consider reveiew it. Thanks.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachments:
fix-idle_session_timeout-missing.patchtext/x-patchDownload
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index f6d0562876..c7070b05c9 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -602,6 +602,7 @@ AutoVacLauncherMain(int argc, char *argv[])
SetConfigOption("lock_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
SetConfigOption("idle_in_transaction_session_timeout", "0",
PGC_SUSET, PGC_S_OVERRIDE);
+ SetConfigOption("idle_session_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
/*
* Force default_transaction_isolation to READ COMMITTED. We don't want
@@ -1624,6 +1625,7 @@ AutoVacWorkerMain(int argc, char *argv[])
SetConfigOption("lock_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
SetConfigOption("idle_in_transaction_session_timeout", "0",
PGC_SUSET, PGC_S_OVERRIDE);
+ SetConfigOption("idle_session_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
/*
* Force default_transaction_isolation to READ COMMITTED. We don't want
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 8903a694ae..15960ea644 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3059,6 +3059,7 @@ _doSetFixedOutputState(ArchiveHandle *AH)
ahprintf(AH, "SET statement_timeout = 0;\n");
ahprintf(AH, "SET lock_timeout = 0;\n");
ahprintf(AH, "SET idle_in_transaction_session_timeout = 0;\n");
+ ahprintf(AH, "SET idle_session_timeout = 0;\n");
/* Select the correct character set encoding */
ahprintf(AH, "SET client_encoding = '%s';\n",
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b52f3ccda2..fd4f7269c3 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1146,6 +1146,8 @@ setup_connection(Archive *AH, const char *dumpencoding,
ExecuteSqlStatement(AH, "SET lock_timeout = 0");
if (AH->remoteVersion >= 90600)
ExecuteSqlStatement(AH, "SET idle_in_transaction_session_timeout = 0");
+ if (AH->remoteVersion >= 140000)
+ ExecuteSqlStatement(AH, "SET idle_session_timeout = 0");
/*
* Quote all identifiers, if requested.
Le jeu. 30 déc. 2021 à 12:01, Japin Li <japinli@hotmail.com> a écrit :
On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge <guillaume@lelarge.info>
wrote:Le jeu. 30 déc. 2021 à 11:44, Japin Li <japinli@hotmail.com> a écrit :
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.Attached fix this, please consider reveiew it. Thanks.
I've read it and it really looks like what I would have done. Sounds good
to me.
--
Guillaume.
Japin Li <japinli@hotmail.com> writes:
On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge <guillaume@lelarge.info> wrote:
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.
Attached fix this, please consider reveiew it. Thanks.
This seems rather pointless to me. The idle-session timeout is only
activated in PostgresMain's input loop, so it will never be reached
in autovacuum or other background workers. (The same is true for
idle_in_transaction_session_timeout, so the fact that somebody made
autovacuum.c clear that looks like cargo-cult programming from here,
not useful code.) And as for pg_dump, how would it ever trigger the
timeout? It's not going to sit there thinking, especially not
outside a transaction.
regards, tom lane
Le jeu. 30 déc. 2021 à 17:25, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
Japin Li <japinli@hotmail.com> writes:
On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge <guillaume@lelarge.info>
wrote:
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.Attached fix this, please consider reveiew it. Thanks.
This seems rather pointless to me. The idle-session timeout is only
activated in PostgresMain's input loop, so it will never be reached
in autovacuum or other background workers. (The same is true for
idle_in_transaction_session_timeout, so the fact that somebody made
autovacuum.c clear that looks like cargo-cult programming from here,
not useful code.) And as for pg_dump, how would it ever trigger the
timeout? It's not going to sit there thinking, especially not
outside a transaction.
Agreed. It makes more sense. So no need for the patch. Thanks to both.
--
Guillaume.
On Fri, 31 Dec 2021 at 00:24, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Japin Li <japinli@hotmail.com> writes:
On Thu, 30 Dec 2021 at 18:53, Guillaume Lelarge <guillaume@lelarge.info> wrote:
pg_dump works in a single transaction, so it's already dealt with
idle_in_transaction_timeout. Though I guess setting both would work too.Attached fix this, please consider reveiew it. Thanks.
This seems rather pointless to me. The idle-session timeout is only
activated in PostgresMain's input loop, so it will never be reached
in autovacuum or other background workers. (The same is true for
idle_in_transaction_session_timeout, so the fact that somebody made
autovacuum.c clear that looks like cargo-cult programming from here,
not useful code.) And as for pg_dump, how would it ever trigger the
timeout? It's not going to sit there thinking, especially not
outside a transaction.
Thanks for your clarify! If the timeout never be reached, should we remove
those settings?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.