Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abandon queries when client disconnects #3867

Closed
rbrw opened this issue Aug 30, 2023 · 1 comment
Closed

Abandon queries when client disconnects #3867

rbrw opened this issue Aug 30, 2023 · 1 comment

Comments

@rbrw
Copy link
Contributor

rbrw commented Aug 30, 2023

Testing revealed that pdb streaming queries definitely do continue running for a while after client disconnection. Long after the client disconnected, jetty would continue to call the query-eng/generated-stream read to stream the encoded json bytes which in turn keep the database query running. The current assumption is that the reads are caused by buffering before the next network write. A stack trace from the read reveals that there's a gzip handler, but since (at least from a cursory investigation) that handler appears to rely on a Deflate instance with a 512 byte buffer (fixed?), it's probably not the key reason for the continuing reads.

In any case, after further investigation, we didn't find a straightforward way to detect a client disconnection in the ring or tk-jetty9 world, but since the jetty/servlet response handler is given a ServletResponse object, which for jetty is an org.eclipse.jetty.server.Response, I wondered if that might provide access to the client connection. It does.

Jetty (9 at least) handles the connection via a chain of "interceptors" attached to the HttpOutput, which is itself attached to the Response. For a trivial jetty app there's just one interceptor, while pdb includes others (the gzip "layer" at least). The jetty HttpOutput Interceptor comments say that the last link in the chain will be an HttpChannel, and for pdb, that's currently an HttpChannelOverHttp instance, and that allows direct access to the jdk SocketChannel connected to the client.

Fortunately the client channel is also in non-blocking mode, and so even though the client isn't (and shouldn't be) sending any data, an attempt to read from the client channel will return 0 as long as the client is connected and -1 after it disconnects (verified via curl against both the simple test app and pdb itself). It's then straightforward to add a check in pdb's query-eng/body-stream's stream-rows that will cause pdb to abandon the query whenever the read indicates that the client is gone.

This approach relies on some assumptions:

The jetty transport is non-blocking.

The read from the client always returns -1 after a disconnect.

By the time pdb is streaming, it's acceptable to attempt to read bytes from the client, i.e. either the client won't be sending any bytes, or it's OK to discard them.

It would also require a change to trapperkeeper-webserver-jetty9, because ring handlers (both in ring itself and in tk-jetty9) don't provide the response instance to the handler, just the request map. One option would be to add an :include-response? option to the add-ring-handler service method. When true the handler call would become (handle request-map response) instead of (handle request-map).

@rbrw
Copy link
Contributor Author

rbrw commented Aug 30, 2023

Implmented via puppetlabs/trapperkeeper-webserver-jetty9#251 and #3837 (PDB-5645)

@rbrw rbrw closed this as completed Aug 30, 2023
@rbrw rbrw changed the title (PDB-5645) Abandon queries when client disconnects Abandon queries when client disconnects Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant