RE: Terminate the idle sessions

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Li Japin' <japinli@hotmail.com>, "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, "bharath.rupireddyforpostgres@gmail.com" <bharath.rupireddyforpostgres@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2020-11-17T06:07:35Z
Lists: pgsql-hackers
Dear Li, David,

> Additionally, using postgres_fdw within the server doesn't cause issues,
> its using postgres_fdw and the remote server having this setting set to zero that causes a problem.

I didn't know the fact that postgres_fdw can use within the server... Thanks.

I read optimize-setitimer patch, and looks basically good. I put what I understanding,
so please confirm it whether your implementation is correct.
(Maybe I missed some simultaneities, so please review anyone...)

[besic consept]

sigalrm_due_at means the time that interval timer will ring, and sigalrm_delivered means who calls schedule_alarm().
If fin_time of active_timeouts[0] is larger than or equal to sigalrm_due_at,
stop calling setitimer because handle_sig_alarm() will be call sooner.

[when call setitimer]

In the attached patch, setitimer() will be only called the following scenarios:

* when handle_sig_alarm() is called due to the pqsignal
* when a timeout is registered and its fin_time is later than active_timeous[0]
* when disable a timeout
* when handle_sig_alarm() is interrupted and rescheduled(?)

According to comments, handle_sig_alarm() may be interrupted because of the ereport.
I think if handle_sig_alarm() is interrupted before subsutituting sigalrm_due_at to true,
interval timer will be never set. Is it correct, or is my assumption wrong?

Lastly, I found that setitimer is obsolete and should change to another one. According to my man page:

```
POSIX.1-2001, SVr4, 4.4BSD (this call first appeared in 4.2BSD).
POSIX.1-2008 marks getitimer() and setitimer() obsolete,
recommending the use of the POSIX timers API (timer_gettime(2), timer_settime(2), etc.) instead.
```

Do you have an opinion for this? I think it should be changed
if all platform can support timer_settime system call, but this fix affects all timeouts,
so more considerations might be needed.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Rethink SQLSTATE code for ERRCODE_IDLE_SESSION_TIMEOUT.

  2. Improve commentary in timeout.c.

  3. Add idle_session_timeout.

  4. Improve timeout.c's handling of repeated timeout set/cancel.