Re: Timeout parameters

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: "Nagaura, Ryohei" <nagaura(dot)ryohei(at)jp(dot)fujitsu(dot)com>
Cc: "'pgsql-hackers(at)postgresql(dot)org'" <pgsql-hackers(at)postgresql(dot)org>, "AYahorau(at)ibagroup(dot)eu" <AYahorau(at)ibagroup(dot)eu>
Subject: Re: Timeout parameters
Date: 2018-11-05 17:08:18
Message-ID: alpine.DEB.2.21.1811041235200.7070@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Ryohei,

> I'd like to suggest introducing two parameters to handle client-server
> communication timeouts.

I'm generally fine with giving more access to low-level parameters to
users. However, I'm not sure I understand the use case you have that needs
these new extensions.

> "socket_timeout" parameter.

About the "socket_timout" patch:

Patch does not apply cleanly because of a "trailing whitespace" in a
comment. Please remove spaces at the end of lines.

I'd like clarifications about the use case that needs this specific
feature, especially to understand why the server-side "statement_timeout"
setting is not right enough.

> "socket_timeout" is the application layer timeout parameter from when
> frontend issues SQL query to when frontend receives the execution result
> from backend. When this parameter is active and timeout occurs, frontend
> close the socket. It is a merit for client to set the maximum time to
> wait for SQL.

I think that there is some kind of a misnomer: this is not a socket-level
timeout, but a client-side query timeout, so it should be named
differently? I'm not sure how to name it, though.

I checked that the feature works at the psql level.

sh> psql "socket_timeout=2"

psql> SELECT 1;
1

psql> SELECT pg_sleep(3);
timeout expired
The connection to the server was lost. Attempting reset: Succeeded.

The timeout is per statement, if there are several statements, each get
its own timeout, just like server-side "statement_timeout".

I think that the way it works is a little extreme, basically the
connection is aborted from within pqWait, and then restarted from scratch.
I would not expect that from such a feature, but I'm not sure how to
cancel a query from libpq, but it is possible, eg:

psql> SELECT pg_sleep(10);
^C Cancel request sent
ERROR: canceling statement due to user request

psql>

Would that be better? It probably assumes that the connection is okay.

The implementation looks awkward, because part of the logic of pqWaitTimed
is reimplemented in pqWait. Also, I do not understand the computation
of finish_time, which seems to assume that pqWait is going to be called
immediately after sending a query, which may or may not be the case, and
if it is not the time() call there is not the start of the statement.

C style: all commas should be followed by a space (or newline).

There is no clear way to know about the value of the setting (SHOW, \set,
\pset...). Ok, this is already the case of other connection parameters.

Using "atoi" is a bad idea because it accepts trailing garbage and does
not detect overflows. Use the "parse_int_param" function instead.

There are no tests.

There is no documentation.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-11-05 17:23:52 Re: pread() and pwrite()
Previous Message Alvaro Herrera 2018-11-05 16:07:30 Re: pread() and pwrite()