Re: Add client connection check during the execution of the query
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: thomas.munro@gmail.com, s.kelvich@postgrespro.ru,
pgsql-hackers@postgresql.org
Date: 2019-07-18T05:04:13Z
Lists: pgsql-hackers
Tatsuo Ishii <ishii@sraoss.co.jp> writes: >> Yeah, the timer logic is wrong. I didn't have time to look into it >> but with truss/strace for some reason I see 3 setitimer() syscalls for >> every query, but I think this doesn't even need to set the timer for >> every query. > Hum. I see 2 settimer(), instead of 3. src/backend/utils/misc/timeout.c is not really designed for there to be timeouts that persist across multiple queries. It can probably be made better, but this patch doesn't appear to have touched any of that logic. To point to just one obvious problem, the error recovery path (sigsetjmp block) in postgres.c does disable_all_timeouts(false); which cancels *all* timeouts. Probably don't want that behavior anymore. I think the issue you're complaining about may have to do with the fact that if there's no statement timeout active, both enable_statement_timeout and disable_statement_timeout will call "disable_timeout(STATEMENT_TIMEOUT, false);". That does nothing, as desired, if there are no other active timeouts ... but if there is one, ie the client_connection timeout, we'll end up calling schedule_alarm which will call setitimer even if the desired time-of-nearest-timeout hasn't changed. That was OK behavior for the set of timeouts that the code was designed to handle, but we're going to need to be smarter now. regards, tom lane
Commits
-
Add WL_SOCKET_CLOSED for socket shutdown events.
- 50e570a59e7f 15.0 landed
-
Use WL_SOCKET_CLOSED for client_connection_check_interval.
- cba5b994c990 15.0 landed
-
Detect POLLHUP/POLLRDHUP while running queries.
- c30f54ad732c 14.0 landed
-
Improve timeout.c's handling of repeated timeout set/cancel.
- 09cf1d522676 14.0 cited