Re: Implementing setQueryTimeout()

Tom Lane wrote:

> That seems pretty darn horrid, actually. If the reason for the slow
> response is server overload, this technique will make things rapidly
> *worse*. In the first place it does nothing to prevent the server from
> continuing to compute the too-slow query (and perhaps even committing
> it). In the second place, having to establish a new connection will eat
> a lot of cycles you really don't want to waste. In the third place,
> once you do establish a new connection it will be competing for cycles
> with the still-running query in the original backend. Iterate a few
> times and you'll have a self-inflicted denial of service.

Except for the problem of the query continuing to run, these problems
seem to be common to anything that throws an exception to the client on
timeout. The client is going to have to give up on that query regardless
of how we actually implement the timeout, so the server is doing extra
"useless" work anyway. If the surrounding logic is not smart enough to
throttle itself, then you're hosed either way.

> I agree with having a timer thread, I think, just not with what you want
> to do when the timer fires. Can't you do something like sending a query
> cancel request when you time out?

I could do that, but if the problem is actually that the server or
network has died it will not help things (the cancel is just going to
fail.. eventually).

Maybe two timeouts? One causes query cancel; if the cancel doesn't
actually happen by the time the second timeout occurs, the connection
gets closed.

In fact that could be done by simply doing both of the approaches I
suggested with different timeouts - set statement_timeout, and if you're
still waiting for a response some time after statement_timeout should
have fired, you kill the connection? This has the advantage that you
don't have to juggle an extra connection to do the cancel.

> It might be that you need to decouple queries from connections a bit
> more, so that a query can fail and "let go" of a connection, while the
> connection object has to wait for its query to be cancelled before
> returning to the pool of available connections.

This is going to be a lot more work than I have time to do unfortunately
:( The main problems are that:

(1) the current code stores a fair amount of protocol state in what are
essentially local variables - so you can't just unwind the stack and
throw an exception to the caller at an arbitary point without losing
important protocol state. The current code is very careful to wait until
it is at a known point in the protocol (ReadyForQuery, IIRC) before
returning. So there would be quite a lot of rework needed here.

(2) there is no simple way to nondestructively interrupt a blocking I/O
call deep in the protocol code; and a rewrite to allow this (a) is a lot
of work and (b) would probably require that we drop support for older
Java versions.

It also seems a bit problematic in this scenario:

- client get connection from pool
- client runs query
- query throws SQLException due to timeout; at the protocol level we are
still waiting for the query to cancel
- client logs exception, cleans up, returns the connection to the pool
- connection gets reused
- new query gets backed up behind the cancelling query

Essentially you've now got a useless connection in the pool.. In fact,
in this scenario it's likely the pool will be running some cleanup SQL
before handing it out to a new client, and that will fail - so the
connection will probably be discarded entirely anyway!

(There is no way in the JDBC API to say to a client or connection pool
"this connection is busy, please don't use it right now")

-O