Repository navigation
feat(qwp): add per-query timeout to the query client - #105
Open
bluestreak01 wants to merge 2 commits into
Open
bluestreak01 wants to merge 2 commits into
bluestreak01 wants to merge 2 commits into
Conversation
Adds a query timeout to QwpQueryClient and the QuestDB facade: query_timeout_ms (connection-string default), Query.timeout(...) per handle, and an execute() overload with an explicit timeout. A timed-out query ends with the new STATUS_QUERY_TIMEOUT (QueryException.isTimeout()) on a connection that stays open and authenticated for the next query. - A server advertising the new CAP_QUERY_TIMEOUT (0x08) capability gets the remaining budget as a timeout_ms field after query_flags (QUERY_FLAG_TIMEOUT, 0x02) and ends the query itself with STATUS_QUERY_TIMEOUT (0x0E). Older servers are cancelled by the client at the deadline; the cancellation is reported as the same status. - Once the deadline passes no further result batch reaches the handler. Discarded batches are still decoded, keeping the connection-scoped SYMBOL dict in step with the server. - If the server does not end the query within the grace period (query_close_timeout_ms on the facade), the caller is released anyway and the connection drains the aborted query in the background. Only a connection that stays silent through a second grace period is closed. - The deadline spans failover: replays carry the remaining budget, the backoff and every reconnect step are bounded by it, and a reconnect it cut short is retried by the next execute() instead of leaving the client disconnected. Pool: Query.close() waits for a worker still draining a timed-out query before returning it, discards a worker whose connection failed, and a cancel issued while a submission waits behind such a drain is no longer lost. The server side (decoding timeout_ms, the breaker timeout, the status mapping and the capability advert) lands in a tandem questdb PR.
bluestreak01
marked this pull request as ready for review
October 7, 2026 13:50
A query that ended early on the client side left its remaining frames on the connection, and the next query read them as its own result. This fixes the three ways in. Handler throws on a query timeout. At the end of the grace period execute() reports the timeout through onError while the aborted query still runs on the connection. If onError threw, execute() skipped the drain that follows. FailoverProbeHandler now holds the exception, executeOnce keeps draining as it does for a handler that returns, and executeImpl rethrows the original throwable once the drain ends or the connection is given up. The facade worker reports an exception that escapes execute() as the outcome of the current submission. Rethrown after the drain, it could land after the caller, already released by the timeout, had submitted again on the same Query, and complete that newer submission with the old exception. QueryImpl now numbers submissions, and runOn() reports an escaping exception only while its own submission is current. The check and the report share doneLock with submit(). This also closes the same race, until now microseconds wide, for a handler that throws from onEnd or onExecDone. Handler throws from onBatch, or the thread is interrupted. The I/O thread now stamps every event of a query with that query's request id; connection-level events carry QueryEvent.ANY_REQUEST and still reach whichever query waits. executeOnce skips the events of other queries, so the next query no longer receives the leftovers of an abandoned one, and it handles its own deadline even while leftovers keep coming. An attempt that ends before its query's last event now cancels that query, so the leftovers stop coming. While the I/O thread still works through an abandoned query, the next query can be queued and cancelled. The I/O thread sent that CANCEL at once, ahead of the query itself, and the server drops a cancel of a query it does not know, so the cancel was lost. drainPendingCancel now sends only a cancel of the query it serves, holds one of a later query, and drops one of a query that has already ended. Interrupt before the I/O thread encodes the request. execute() returned while the I/O thread could still read the request holder, the bind buffer and the caller's SQL text, and the next execute() overwrote them: the interrupted query was never sent and the next one was sent twice, possibly mixing the two queries' SQL and binds. With leftovers of an abandoned query on the connection, the next execute() could also block forever on the single request slot, while the I/O thread waited for it to release a leftover batch. An interrupted execute() now withdraws a request the I/O thread has not picked up yet, or waits for the encoding the I/O thread has started, before it reports the interrupt. An I/O thread still not done after shutdownJoinMs gives up the connection, as on a query timeout. Behavior visible to callers: an exception from onError at the end of the grace period now surfaces when the drain ends, up to one more grace period later, which is when execute() already returns for a handler that does not throw. A handler that throws from onBatch, or an interrupt, now cancels its query, and the next query skips that query's leftovers instead of returning them. A handler that does not throw sees no change, and the per-batch path gains only one field store on the I/O thread and one comparison on the reading thread. New tests drive each path against the scripted mock server, through both QwpQueryClient and the QuestDB facade.
Contributor
[PR Coverage check]😍 pass : 306 / 356 (85.96%) file detail
|
This branch has not been deployed
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
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.
Tandem: questdb/questdb#7768 (server side). It decodes
timeout_ms, applies it to the query's circuit breaker, reports timeouts asSTATUS_QUERY_TIMEOUT, and advertisesCAP_QUERY_TIMEOUT. Against servers without it, this client enforces the timeout on its own (client deadline +CANCEL).Merge order: this PR first. questdb/questdb#7768 pins the submodule to this branch's head and moves the pointer to the merged commit before it merges. Both sides use the same protocol constants:
CAP_QUERY_TIMEOUT = 0x08,QUERY_FLAG_TIMEOUT = 0x02,STATUS_QUERY_TIMEOUT = 0x0E.What
Queries can now be given a timeout:
query_timeout_ms=Nin the connection string sets a default for every query (0, the default, means none);Query.timeout(long, TimeUnit)overrides it per handle;QwpQueryClient.withQueryTimeout(long)sets the default andexecute(sql, binds, handler, resetSymbolDict, timeoutMs)overrides it per call.A timed-out query fails with
QueryException.isTimeout()(status0x0E), and the pooled, authenticated connection stays open for the next query.Completion.await(timeout)keeps its meaning: it only bounds the wait. Its javadoc now points to the query timeout.How
Wire. When
SERVER_INFOadvertisesCAP_QUERY_TIMEOUT, the client setsQUERY_FLAG_TIMEOUTand appendstimeout_ms:varint(the remaining budget) afterquery_flags. The server applies it to the query'sNetworkSqlExecutionCircuitBreaker, the same mechanism as HTTP'sStatement-Timeout, which trips inside the SQL engine. It then ends the query with aQUERY_ERROR. That is a per-query error, so the connection is kept. Without the capability, frames are byte-identical to today.Client deadline (
QwpQueryClient.executeOnce), always on:CANCEL, and itsSTATUS_CANCELLEDis reported asSTATUS_QUERY_TIMEOUT, unless the user had cancelled first.The grace period is
query_close_timeout_mson the facade (default 5 s) andwithQueryTimeoutGraceon the low-level client.Failover. One deadline spans every attempt:
SERVER_INFO) are bounded by it, so a black-holed endpoint can't hold the caller past the timeout;execute().Behaviour change: the last point also covers failover reconnects that fail for any non-auth reason. They previously left the client permanently "not connected" (every later
execute()threwIllegalStateException), which is now reachable just by a short timeout cutting the reconnect off.Pool.
Query.close()now waits, withinquery_close_timeout_ms, for a worker that is still draining a timed-out query before returning it to the pool.execute()clears the client's cancel latch on entry. The handle now remembers the cancel and re-applies it before the request is sent.The per-submit path allocates nothing: the deadline state is primitives, and
QwpSpscQueue.take(deadline)usesparkNanos. Java 8 APIs only.API notes:
Querygains an abstracttimeout(long, TimeUnit), which breaks compilation only for third-party implementations of the interface. The internalQwpEgressIoThread.submitQuerytakes an extratimeoutMsargument.Test plan
QwpQueryClientQueryTimeoutTest(13 tests, scripted mock server):QueryTimeoutFacadeTest(5 tests): the configured default and its per-handle override; the connection kept (one handshake); a draining worker returned only once idle; an unresponsive worker replaced; a cancel of a queued submission not lost.QwpSpscQueueTest(timed take),QueryCloseDrainTest(busy worker),QueryImplResetTest,QwpQueryClientConfigHonoredTest.mvn -P javadoc -DskipTests clean packagepasses on JDK 8 (withJAVA11_HOME) and JDK 25, with no new javadoc warnings.