Thread: Autovacuum and idle_session_timeout

Autovacuum and idle_session_timeout

From
Guillaume Lelarge
Date:
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.

Re: Autovacuum and idle_session_timeout

From
Japin Li
Date:
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.



Re: Autovacuum and idle_session_timeout

From
Guillaume Lelarge
Date:
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.

Re: Autovacuum and idle_session_timeout

From
Japin Li
Date:
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.

Attachment

Re: Autovacuum and idle_session_timeout

From
Guillaume Lelarge
Date:
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.

Re: Autovacuum and idle_session_timeout

From
Tom Lane
Date:
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



Re: Autovacuum and idle_session_timeout

From
Guillaume Lelarge
Date:
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.

Re: Autovacuum and idle_session_timeout

From
Japin Li
Date:
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.