Autovacuum and idle_session_timeout

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 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?

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.

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.