Hello Ryohei-san,
There are three independent patches in this thread.
About the socket timeout patch v7:
Patch applies & compiles cleanly. "make check" is ok, although there are
no specific tests, which is probably ok. Doc build is ok.
I'd simplify the doc first sentence to:
""" Number of seconds to wait for socket read/write operations before
closing the connection, as a decimal integer. This can be used both to
force global client-side query timeout and to detect network problems. A
value of zero (the default) turns this off, which means wait indefinitely.
The minimum allowed timeout is 2 seconds, so a value of 1 is interpreted
as 2."""
The code looks mostly ok.
The comment on finish_time computation does not look very useful to me, it
just states in English the simple formula below. I'd suggest to remove it.
I've tested setting "socket_timeout=2" and doing "SELECT pg_sleep(10);".
It works somehow, however after a few attempts I have:
psql> SELECT COUNT(*) FROM pg_stat_activity WHERE application_name='psql';
3
I.e. I have just created lingering postmasters, which only stop when the
queries are actually finished. I cannot say that I'm thrilled by that
behavior. ISTM that libpq should at least attempt to cancel the query by
sending a cancel request before closing the connection, as I have already
suggested in a previous review. The "client side query timeout" use case
does not seem well addressed by the current implementation.
If the user reconnects, eg "\c db", the setting is lost. The re-connection
handling should probably take care of this parameter, and maybe others.
The implementation does not check that the parameter is indeed an integer,
eg "socket_timeout=2bad" is silently ignored. I'd suggest to check like
already done for other parameters, eg "port".
--
Fabien.
