Send heartbeats while the caller consumes fetched rows - #632
Open
hashhar wants to merge 3 commits into
Open
Conversation
hashhar
force-pushed
the
hashhar/463-query-stuck-finishing
branch
from
August 19, 2026 21:40
2cd7679 to
904d56e
Compare
hashhar
changed the title
The client made no requests between fetch() calls. A caller holding a batch of rows for longer than query.client.timeout got the query abandoned by the coordinator. Run the heartbeat between fetch() calls. It stops on cancel and completion, and exits when the TrinoQuery is garbage collected. Remove the per-download heartbeat wiring, the query-level heartbeat covers segment downloads.
The unit tests slept for a fixed 0.1s and asserted heartbeat call counts, which is racy on loaded machines. Serve scripted HEAD responses from a recording request, wait on a semaphore and join the heartbeat thread instead. The spooled download integration test polled the coordinator's http-request.log for up to 9s. Count HEAD requests client-side through the http_session.
SegmentIterator took paired request and heartbeat_interval arguments and validated the pairing itself, while TrinoQuery dug the interval out of private session state. Add TrinoRequest.heartbeat() returning a configured heartbeat or None when disabled. TrinoQuery is the only caller. Drop SegmentIterator's heartbeat parameters and the unused context-manager protocol on _RequestHeartbeat.
hashhar
force-pushed
the
hashhar/463-query-stuck-finishing
branch
from
August 20, 2026 13:27
459267d to
46d44b8
Compare
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.
Description
Fixes #463.
The client makes no requests between
fetch()calls. A caller holding a batch of rows for longer thanquery.client.timeoutgets the query abandoned by the coordinator.TrinoQuerynow runs_RequestHeartbeatbetweenfetch()calls.fetch()stops it on entry and restarts it before returning, so the heartbeat never overlaps a request on the same HTTP session. The heartbeat stops on completion andcancel(), and exits via a weakref check when theTrinoQueryis garbage collected.The per-download heartbeat wiring is removed, the query-level heartbeat covers segment downloads. Follow-up commits make the pre-existing heartbeat tests deterministic (semaphore waits and thread joins instead of sleeps, client-side HEAD counting instead of polling server logs) and move heartbeat construction behind
TrinoRequest.heartbeat(), droppingSegmentIterator'srequest/heartbeat_intervalparameters so the request layer is the single owner of the mechanism.heartbeat_interval(default 30s) controls the interval,Nonedisables it. Servers without HEAD support on the statement endpoint (older than Trino 475) keep the old abandonment behavior.Non-technical explanation
The client pings the coordinator between fetches, so slow consumers no longer fail with "Query was abandoned by the client".
Release notes
( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: