Terminate the idle sessions

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
44 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Terminate the idle sessions

Li Japin
Hi, hackers

When some clients connect to database in idle state, postgres do not close the idle sessions,
here i add a new GUC idle_session_timeout to let postgres close the idle sessions, it samilar
to idle_in_transaction_session_timeout.

Best, regards.

Japin Li


0001-Allow-terminating-the-idle-sessions.patch (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

David G Johnston
On Tuesday, June 9, 2020, Li Japin <[hidden email]> wrote:
Hi, hackers

When some clients connect to database in idle state, postgres do not close the idle sessions,
here i add a new GUC idle_session_timeout to let postgres close the idle sessions, it samilar
to idle_in_transaction_session_timeout

I’m curious as to the use case because I cannot imagine using this.  Idle connections are normal.  Seems better to monitor them and conditionally execute the disconnect backend function from the monitoring layer than indiscriminately disconnect based upon time.  Though i do see an interesting case for attaching to specific login user accounts that only manually login and want the equivalent of a timed screen lock.

David J.
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin


On Jun 9, 2020, at 10:35 PM, David G. Johnston <[hidden email]> wrote:

I’m curious as to the use case because I cannot imagine using this.  Idle connections are normal.  Seems better to monitor them and conditionally execute the disconnect backend function from the monitoring layer than indiscriminately disconnect based upon time. 

I agree with you.  But we can also give the user to control the idle sessions lifetime.
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Michael Paquier-2
On Wed, Jun 10, 2020 at 05:20:36AM +0000, Li Japin wrote:
> I agree with you.  But we can also give the user to control the idle
> sessions lifetime.

Idle sessions staying around can be a problem in the long run as they
impact snapshot building.  You could for example use a background
worker to do this work, like that:
https://github.com/michaelpq/pg_plugins/tree/master/kill_idle
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin


On Jun 10, 2020, at 4:25 PM, Michael Paquier <[hidden email]> wrote:

Idle sessions staying around can be a problem in the long run as they
impact snapshot building.  You could for example use a background
worker to do this work, like that:
https://github.com/michaelpq/pg_plugins/tree/master/kill_idle

Why not implement it in the core of Postgres? Are there any disadvantages of
implementing it in the core of Postgres?

Japin Li
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Adam Brusselback
> Why not implement it in the core of Postgres? Are there any disadvantages of
> implementing it in the core of Postgres?  
I was surprised this wasn't a feature when I looked into it a couple years ago. I'd use it if it were built in, but I am not installing something extra just for this.

> I’m curious as to the use case because I cannot imagine using this.

My use case is, I have a primary application that connects to the DB, most users work through that (setting is useless for this scenario, app manages it's connections well enough). I also have a number of internal users who deal with data ingestion and connect to the DB directly to work, and those users sometimes leave query windows open for days accidentally. Generally not an issue, but would be nice to be able to time those connections out. 

Just my $0.02, but I am +1.
-Adam
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin


On Jun 10, 2020, at 10:27 PM, Adam Brusselback <[hidden email]> wrote:

My use case is, I have a primary application that connects to the DB, most users work through that (setting is useless for this scenario, app manages it's connections well enough). I also have a number of internal users who deal with data ingestion and connect to the DB directly to work, and those users sometimes leave query windows open for days accidentally. Generally not an issue, but would be nice to be able to time those connections out. 

If there is no big impact, I think we might add it builtin.

Japin Li
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Cary Huang-2
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

I applied this patch to the PG13 branch and generally this feature works as described. The new "idle_session_timeout" that controls the idle session disconnection is not in the default postgresql.conf and I think it should be included there with default value 0, which means disabled.
There is currently no enforced minimum value for "idle_session_timeout" (except for value 0 for disabling the feature), so user can put any value larger than 0 and it could be very small like 500 or even 50 millisecond, this would make any psql connection to disconnect shortly after it has connected, which may not be ideal. Many systems I have worked with have 30 minutes inactivity timeout by default, and I think it would be better and safer to enforce a reasonable minimum timeout value
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

David G Johnston
On Mon, Aug 10, 2020 at 2:43 PM Cary Huang <[hidden email]> wrote:
There is currently no enforced minimum value for "idle_session_timeout" (except for value 0 for disabling the feature), so user can put any value larger than 0 and it could be very small like 500 or even 50 millisecond, this would make any psql connection to disconnect shortly after it has connected, which may not be ideal. Many systems I have worked with have 30 minutes inactivity timeout by default, and I think it would be better and safer to enforce a reasonable minimum timeout value

I'd accept a value of say 1,000 being minimum in order to reinforce the fact that a unit-less input, while possible, is taken to be milliseconds and such small values most likely mean the user has made a mistake.  I would not choose a minimum allowed value solely based on our concept of "reasonable".  I don't imagine a value of say 10 seconds, while seemingly unreasonable, is going to be unsafe.

David J.

Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin
In reply to this post by Cary Huang-2
Hi,

On Aug 11, 2020, at 5:42 AM, Cary Huang <[hidden email]> wrote:

I applied this patch to the PG13 branch and generally this feature works as described. The new "idle_session_timeout" that controls the idle session disconnection is not in the default postgresql.conf and I think it should be included there with default value 0, which means disabled. 

Thanks for looking at it!

I’ve attached a new version that add “idle_session_timeout” in the default postgresql.conf. 

On Mon, Aug 10, 2020 at 2:43 PM Cary Huang <[hidden email]> wrote:
There is currently no enforced minimum value for "idle_session_timeout" (except for value 0 for disabling the feature), so user can put any value larger than 0 and it could be very small like 500 or even 50 millisecond, this would make any psql connection to disconnect shortly after it has connected, which may not be ideal. Many systems I have worked with have 30 minutes inactivity timeout by default, and I think it would be better and safer to enforce a reasonable minimum timeout value

I'd accept a value of say 1,000 being minimum in order to reinforce the fact that a unit-less input, while possible, is taken to be milliseconds and such small values most likely mean the user has made a mistake.  I would not choose a minimum allowed value solely based on our concept of "reasonable".  I don't imagine a value of say 10 seconds, while seemingly unreasonable, is going to be unsafe.

I think David is right, see “idle_in_transaction_session_timeout”, it also doesn’t have a “reasonable” minimum value.


v2-0001-Allow-terminating-the-idle-sessions.patch (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Bharath Rupireddy
On Tue, Aug 11, 2020 at 8:45 AM Li Japin <[hidden email]> wrote:
>
> I’ve attached a new version that add “idle_session_timeout” in the default postgresql.conf.
>

Hi, I would like to just mention a use case I thought of while discussing [1]:

In postgres_fdw: assuming we use idle_in_session_timeout on remote
backends,  the remote sessions will be closed after timeout, but the
locally cached connection cache entries still exist and become stale.
The subsequent queries that may use the cached connections will fail,
of course these subsequent queries can retry the connections only at
the beginning of a remote txn but not in the middle of a remote txn,
as being discussed in [2]. For instance, in a long running local txn,
let say we used a remote connection at the beginning of the local
txn(note that it will open a remote session and it's entry is cached
in local connection cache), only we use the cached connection later at
some point in the local txn, by then let say the
idle_in_session_timeout has happened on the remote backend and the
remote session would have been closed, the long running local txn will
fail instead of succeeding.

I think, since the idle_session_timeout is by default disabled, we
have no problem. My thought is what if a user enables the
feature(knowingly or unknowingly) on the remote backend? If the user
knows about the above scenario, that may be fine. On the other hand,
either we can always the feature on the remote backend(at the
beginning of the remote txn, like we set for some other configuration
settings see - configure_remote_session() in connection.c) or how
about mentioning the above scenario in this feature documentation?

[1] - https://www.postgresql.org/message-id/CALj2ACU1NBQo9mihA15dFf6udkOi7m0u2_s5QJ6dzk%3DZQyVbwQ%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/flat/CALj2ACUAi23vf1WiHNar_LksM9EDOWXcbHCo-fD4Mbr1d%3D78YQ%40mail.gmail.com

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin

On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <[hidden email]> wrote:

I think, since the idle_session_timeout is by default disabled, we
have no problem. My thought is what if a user enables the
feature(knowingly or unknowingly) on the remote backend? If the user
knows about the above scenario, that may be fine. On the other hand,
either we can always the feature on the remote backend(at the
beginning of the remote txn, like we set for some other configuration
settings see - configure_remote_session() in connection.c) or how
about mentioning the above scenario in this feature documentation?

Though we can disable the idle_session_timeout when using postgres_fdw,
there still has locally cached connection cache entries when the remote sessions
terminated by accident.  AFAIK, you have provided a patch to solve this
problem, and it is in current CF [1].


Best Regards,
Japin Li.
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Bharath Rupireddy
On Fri, Aug 14, 2020 at 1:32 PM Li Japin <[hidden email]> wrote:

>
> On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <[hidden email]> wrote:
>
> I think, since the idle_session_timeout is by default disabled, we
> have no problem. My thought is what if a user enables the
> feature(knowingly or unknowingly) on the remote backend? If the user
> knows about the above scenario, that may be fine. On the other hand,
> either we can always the feature on the remote backend(at the
> beginning of the remote txn, like we set for some other configuration
> settings see - configure_remote_session() in connection.c) or how
> about mentioning the above scenario in this feature documentation?
>
> Though we can disable the idle_session_timeout when using postgres_fdw,
> there still has locally cached connection cache entries when the remote sessions
> terminated by accident.  AFAIK, you have provided a patch to solve this
> problem, and it is in current CF [1].
>
> [1] - https://commitfest.postgresql.org/29/2651/
>

Yes, that solution can retry the cached connections at only the beginning of the remote txn and not at the middle of the remote txn and that makes sense as we can not retry connecting to a different remote backend in the middle of a remote txn.

+1 for disabling the idle_session_timeout feature in case of postgres_fdw. This can avoid the remote backends to timeout during postgres_fdw usages.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Kyotaro Horiguchi-4
Hello.

At Mon, 17 Aug 2020 19:28:10 +0530, Bharath Rupireddy <[hidden email]> wrote in

> On Fri, Aug 14, 2020 at 1:32 PM Li Japin <[hidden email]> wrote:
> >
> > On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <
> [hidden email]> wrote:
> >
> > I think, since the idle_session_timeout is by default disabled, we
> > have no problem. My thought is what if a user enables the
> > feature(knowingly or unknowingly) on the remote backend? If the user
> > knows about the above scenario, that may be fine. On the other hand,
> > either we can always the feature on the remote backend(at the
> > beginning of the remote txn, like we set for some other configuration
> > settings see - configure_remote_session() in connection.c) or how
> > about mentioning the above scenario in this feature documentation?
> >
> > Though we can disable the idle_session_timeout when using postgres_fdw,
> > there still has locally cached connection cache entries when the remote
> sessions
> > terminated by accident.  AFAIK, you have provided a patch to solve this
> > problem, and it is in current CF [1].
> >
> > [1] - https://commitfest.postgresql.org/29/2651/
> >
>
> Yes, that solution can retry the cached connections at only the beginning
> of the remote txn and not at the middle of the remote txn and that makes
> sense as we can not retry connecting to a different remote backend in the
> middle of a remote txn.
>
> +1 for disabling the idle_session_timeout feature in case of postgres_fdw.
> This can avoid the remote backends to timeout during postgres_fdw usages.

The same already happens for idle_in_transaction_session_timeout and
we can use "ALTER ROLE/DATABASE SET" to dislable or loosen them, it's
a bit cumbersome, though. I don't think we should (at least
implicitly) disable those timeouts ad-hockerly for postgres_fdw.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin

On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <[hidden email]> wrote:

The same already happens for idle_in_transaction_session_timeout and
we can use "ALTER ROLE/DATABASE SET" to dislable or loosen them, it's
a bit cumbersome, though. I don't think we should (at least
implicitly) disable those timeouts ad-hockerly for postgres_fdw.

+1.

Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Thomas Munro-5
On Tue, Aug 18, 2020 at 2:13 PM Li Japin <[hidden email]> wrote:
> On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <[hidden email]> wrote:
> The same already happens for idle_in_transaction_session_timeout and
> we can use "ALTER ROLE/DATABASE SET" to dislable or loosen them, it's
> a bit cumbersome, though. I don't think we should (at least
> implicitly) disable those timeouts ad-hockerly for postgres_fdw.
>
> +1.

This seems like a reasonable feature to me.

The delivery of the error message explaining what happened is probably
not reliable, so to some clients and on some operating systems this
will be indistinguishable from a dropped network connection or other
error, but that's OK and we already have that problem with the
existing timeout-based disconnection feature.

The main problem I have with it is the high frequency setitimer()
calls.  If you enable both statement_timeout and idle_session_timeout,
then we get up to huge number of system calls, like the following
strace -c output for a few seconds of one backend under pgbench -S
workload shows:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 39.45    0.118685           0    250523           setitimer
 29.98    0.090200           0    125275           sendto
 24.30    0.073107           0    126235       973 recvfrom
  6.01    0.018068           0     20950           pread64
  0.26    0.000779           0       973           epoll_wait
------ ----------- ----------- --------- --------- ----------------
100.00    0.300839                523956       973 total

There's a small but measurable performance drop from this, as also
discussed in another thread about another kind of timeout[1].  Maybe
we should try to fix that with something like the attached?

[1] https://www.postgresql.org/message-id/flat/77def86b27e41f0efcba411460e929ae%40postgrespro.ru

0001-Optimize-setitimer-usage.patchx (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin


On Aug 31, 2020, at 8:51 AM, Thomas Munro <[hidden email]> wrote:

The main problem I have with it is the high frequency setitimer()
calls.  If you enable both statement_timeout and idle_session_timeout,
then we get up to huge number of system calls, like the following
strace -c output for a few seconds of one backend under pgbench -S
workload shows:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
39.45    0.118685           0    250523           setitimer
29.98    0.090200           0    125275           sendto
24.30    0.073107           0    126235       973 recvfrom
 6.01    0.018068           0     20950           pread64
 0.26    0.000779           0       973           epoll_wait
------ ----------- ----------- --------- --------- ----------------
100.00    0.300839                523956       973 total

Hi, Thomas,

Could you give the more details about the test instructions?
Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Thomas Munro-5
On Mon, Aug 31, 2020 at 2:40 PM Li Japin <[hidden email]> wrote:
> Could you give the more details about the test instructions?

Hi Japin,

Sure.  Because I wasn't trying to get reliable TPS number or anything,
I just used a simple short read-only test with one connection, like
this:

pgbench -i -s10 postgres
pgbench -T60 -Mprepared -S postgres

Then I looked for the active backend and ran strace -c -p XXX for a
few seconds and hit ^C to get the counters.  I doubt the times are
very accurate, but the number of calls is informative.

If you do that on a server running with -c statement_timeout=10s, you
see one setitimer() per transaction.  If you also use -c
idle_session_timeout=10s at the same time, you see two.


Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Kyotaro Horiguchi-4
In reply to this post by Thomas Munro-5
At Mon, 31 Aug 2020 12:51:20 +1200, Thomas Munro <[hidden email]> wrote in

> On Tue, Aug 18, 2020 at 2:13 PM Li Japin <[hidden email]> wrote:
> > On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <[hidden email]> wrote:
> > The same already happens for idle_in_transaction_session_timeout and
> > we can use "ALTER ROLE/DATABASE SET" to dislable or loosen them, it's
> > a bit cumbersome, though. I don't think we should (at least
> > implicitly) disable those timeouts ad-hockerly for postgres_fdw.
> >
> > +1.
>
> This seems like a reasonable feature to me.
>
> The delivery of the error message explaining what happened is probably
> not reliable, so to some clients and on some operating systems this
> will be indistinguishable from a dropped network connection or other
> error, but that's OK and we already have that problem with the
> existing timeout-based disconnection feature.
>
> The main problem I have with it is the high frequency setitimer()
> calls.  If you enable both statement_timeout and idle_session_timeout,
> then we get up to huge number of system calls, like the following
> strace -c output for a few seconds of one backend under pgbench -S
> workload shows:
>
> % time     seconds  usecs/call     calls    errors syscall
> ------ ----------- ----------- --------- --------- ----------------
>  39.45    0.118685           0    250523           setitimer
>  29.98    0.090200           0    125275           sendto
>  24.30    0.073107           0    126235       973 recvfrom
>   6.01    0.018068           0     20950           pread64
>   0.26    0.000779           0       973           epoll_wait
> ------ ----------- ----------- --------- --------- ----------------
> 100.00    0.300839                523956       973 total
>
> There's a small but measurable performance drop from this, as also
> discussed in another thread about another kind of timeout[1].  Maybe
> we should try to fix that with something like the attached?
>
> [1] https://www.postgresql.org/message-id/flat/77def86b27e41f0efcba411460e929ae%40postgrespro.ru

I think it's worth doing. Maybe we can get rid of doing anything other
than removing an entry in the case where we disable a non-nearest
timeout.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: Terminate the idle sessions

Li Japin
In reply to this post by Thomas Munro-5


> On Aug 31, 2020, at 11:43 AM, Thomas Munro <[hidden email]> wrote:
>
> On Mon, Aug 31, 2020 at 2:40 PM Li Japin <[hidden email]> wrote:
>> Could you give the more details about the test instructions?
>
> Hi Japin,
>
> Sure.  Because I wasn't trying to get reliable TPS number or anything,
> I just used a simple short read-only test with one connection, like
> this:
>
> pgbench -i -s10 postgres
> pgbench -T60 -Mprepared -S postgres
>
> Then I looked for the active backend and ran strace -c -p XXX for a
> few seconds and hit ^C to get the counters.  I doubt the times are
> very accurate, but the number of calls is informative.
>
> If you do that on a server running with -c statement_timeout=10s, you
> see one setitimer() per transaction.  If you also use -c
> idle_session_timeout=10s at the same time, you see two.
Hi, Thomas,

Thanks for your point out this problem, here is the comparison.

Without Optimize settimer usage:
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 41.22    1.444851           1   1317033           setitimer
 28.41    0.995936           2    658622           sendto
 24.63    0.863316           1    659116       599 recvfrom
  5.71    0.200275           2    111055           pread64
  0.03    0.001152           2       599           epoll_wait
  0.00    0.000000           0         1           epoll_ctl
------ ----------- ----------- --------- --------- ----------------
100.00    3.505530               2746426       599 total

With Optimize settimer usage:
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 49.89    1.464332           1   1091429           sendto
 40.83    1.198389           1   1091539       219 recvfrom
  9.26    0.271890           1    183321           pread64
  0.02    0.000482           2       214           epoll_wait
  0.00    0.000013           3         5           setitimer
  0.00    0.000010           2         5           rt_sigreturn
  0.00    0.000000           0         1           epoll_ctl
------ ----------- ----------- --------- --------- ----------------
100.00    2.935116               2366514       219 total

Here’s a modified version of Thomas’s patch.


v3-0001-Allow-terminating-the-idle-sessions.patch (12K) Download Attachment
v3-0002-Optimize-setitimer-usage.patch (5K) Download Attachment
123