Add spooling query heartbeat - #179
Open
Flgado wants to merge 1 commit into
Open
Conversation
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During the
spooling protocol, send periodic HTTP HEAD requests to the current nextUri so the coordinator still sees an active client while results are consumed slowly.Configure interval on the connection: DSN
heartbeat_intervaland Config.HeartbeatInterval (positive time.ParseDuration); if omitted, default 30s when spooling starts.Stop heartbeating on 404/405 (unsupported) or after repeated failures; tie lifecycle to statement Close (doneCh + WaitGroup), with a short per-request timeout for HEAD.
Tests: spooled mock server + HEAD counting; DSN validation (invalid / zero / negative / valid); format round-trip; Ping path for bad DSN.
Docs: README spooling section + DSN parameter list.
Integration tests
I did not add an integration test in the same style as the Java client change in trinodb/trino#25267.
That PR can drive a small
query.client.timeouton a test-only Trino server so slow spooled consumption reliably hits “query abandoned” unless heartbeats run. Here, our integration suite talks to a normal shared Trino (Docker in CI).query.client.timeoutis coordinator-wide, not something we can set per connection or per query from the Go client, so we cannot cheaply reproduce the same “tight timeout” scenario without extra Trino config / a dedicated container / a new CI profile. Any idea for this @nineinchnick ?So this PR validates heartbeats with unit tests (mock server, count HEAD to the current nextUri, DSN parsing, etc.), which still checks the client behavior without requiring a special server setup.