Terminate the idle sessions

On Tuesday, June 9, 2020, Li Japin <japinli@hotmail.com> 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.

On Jun 9, 2020, at 10:35 PM, David G. Johnston <david.g.johnston@gmail.com<mailto:david.g.johnston@gmail.com>> 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.

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

On Jun 10, 2020, at 4:25 PM, Michael Paquier <michael@paquier.xyz<mailto:michael@paquier.xyz>> 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

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

On Jun 10, 2020, at 10:27 PM, Adam Brusselback <adambrusselback@gmail.com<mailto:adambrusselback@gmail.com>> 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

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

On Mon, Aug 10, 2020 at 2:43 PM Cary Huang <cary.huang@highgo.ca> 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.

Hi,

On Aug 11, 2020, at 5:42 AM, Cary Huang <cary.huang@highgo.ca<mailto:cary.huang@highgo.ca>> 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 <cary.huang@highgo.ca<mailto:cary.huang@highgo.ca>> 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.

On Tue, Aug 11, 2020 at 8:45 AM Li Japin <japinli@hotmail.com> 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]/messages/by-id/CALj2ACU1NBQo9mihA15dFf6udkOi7m0u2_s5QJ6dzk=ZQyVbwQ@mail.gmail.com:

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]/messages/by-id/CALj2ACUAi23vf1WiHNar_LksM9EDOWXcbHCo-fD4Mbr1d=78YQ@mail.gmail.com. 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]: /messages/by-id/CALj2ACU1NBQo9mihA15dFf6udkOi7m0u2_s5QJ6dzk=ZQyVbwQ@mail.gmail.com
[2]: /messages/by-id/CALj2ACUAi23vf1WiHNar_LksM9EDOWXcbHCo-fD4Mbr1d=78YQ@mail.gmail.com

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

On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com<mailto:bharath.rupireddyforpostgres@gmail.com>> 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]https://commitfest.postgresql.org/29/2651/.

[1]: https://commitfest.postgresql.org/29/2651/

Best Regards,
Japin Li.

On Fri, Aug 14, 2020 at 1:32 PM Li Japin <japinli@hotmail.com> wrote:

On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <

bharath.rupireddyforpostgres@gmail.com> 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

Hello.

At Mon, 17 Aug 2020 19:28:10 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in

On Fri, Aug 14, 2020 at 1:32 PM Li Japin <japinli@hotmail.com> wrote:

On Aug 14, 2020, at 2:15 PM, Bharath Rupireddy <

bharath.rupireddyforpostgres@gmail.com> 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

On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <horikyota.ntt@gmail.com<mailto:horikyota.ntt@gmail.com>> 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.

On Tue, Aug 18, 2020 at 2:13 PM Li Japin <japinli@hotmail.com> wrote:

On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <horikyota.ntt@gmail.com> 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]/messages/by-id/77def86b27e41f0efcba411460e929ae@postgrespro.ru. Maybe
we should try to fix that with something like the attached?

[1]: /messages/by-id/77def86b27e41f0efcba411460e929ae@postgrespro.ru

On Aug 31, 2020, at 8:51 AM, Thomas Munro <thomas.munro@gmail.com<mailto:thomas.munro@gmail.com>> 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?

On Mon, Aug 31, 2020 at 2:40 PM Li Japin <japinli@hotmail.com> 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.

At Mon, 31 Aug 2020 12:51:20 +1200, Thomas Munro <thomas.munro@gmail.com> wrote in

On Tue, Aug 18, 2020 at 2:13 PM Li Japin <japinli@hotmail.com> wrote:

On Aug 18, 2020, at 9:19 AM, Kyotaro Horiguchi <horikyota.ntt@gmail.com> 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] /messages/by-id/77def86b27e41f0efcba411460e929ae@postgrespro.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

On Aug 31, 2020, at 11:43 AM, Thomas Munro <thomas.munro@gmail.com> wrote:

On Mon, Aug 31, 2020 at 2:40 PM Li Japin <japinli@hotmail.com> 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.