feat(core): add local durable acknowledgements to the QWP sender - #98
Open
eugenels wants to merge 4 commits into
Open
feat(core): add local durable acknowledgements to the QWP sender#98eugenels wants to merge 4 commits into
eugenels wants to merge 4 commits into
Conversation
The sender can now request durable acks by tier set instead of a single boolean. request_durable_ack accepts on, off, local, replicated and local,replicated; "on" keeps its shipped meaning as the legacy alias for the replicated tier and still travels as the literal "true" header value with the historical "enabled" confirmation, so nothing changes on the wire for existing configurations. DurableAckTiers encodes the set as a bitmask and owns the header token and confirmation-token mapping. WebSocketClient sends the requested token and validates that the server echoes the granted set verbatim -- the grant is all-or-nothing, a partial or foreign echo counts as a denial and fails the connect with the existing mismatch error. WebSocketResponse recognizes the new STATUS_LOCAL_DURABLE_ACK (0x0E) frame, which shares the STATUS_DURABLE_ACK payload layout. The send loop trims its store-and-forward copy on the strongest requested tier's ack: local-only requests trim on the local ack through the same watermark and drain path; requests including the replicated tier keep trimming on STATUS_DURABLE_ACK and record local acks as progress signals in a separate watermark map. The loop exposes getTotalLocalDurableAcks() and the sender exposes its send loop to tests so server-side integration tests can assert the ack and trim counters end to end.
ExportedApiCompatibilityTest rightly rejected retyping the public
QwpWebSocketSender.connect overloads' durable-ack parameter from
boolean to the DurableAckTiers bitmask: callers compiled against
earlier releases would break with NoSuchMethodError. The nine
pre-branch boolean signatures return as delegating overloads that map
true to the legacy "true" request (replicated tier, "enabled"
confirmation) -- the shipped meaning of the opt-in.
WsSenderConfigHonoredTest now expects the config-token string the
snapshot reports ("on", "local", "replicated", "local,replicated")
and covers the new values. The JVM-error cleanup test follows the
renamed durableAckTiers field in its reflective setup.
Review found localDurableTableWatermarks written but never readable, making the promised both-tier progress signal unreachable. The loop now exposes getLocalDurableTableWatermark(tableName) alongside the other ack counters.
The tier feature landed with only mechanical test edits (false ->
DurableAckTiers.NONE signature updates) and four dead test hooks:
cursorSendLoopForTest, localDurableAck, getTotalLocalDurableAcks and
getLocalDurableTableWatermark had no callers. This commit adds the
missing behavioral coverage and puts every hook to use:
- DurableAckTiersTest covers parseConfigValue/configValue round trips,
the request and confirmation wire tokens, the rejected spellings,
and the isTrimOnLocalAck trim-trigger predicate.
- CursorWebSocketSendLoopDurableAckTest gains local-tier tests:
local-only trim on STATUS_LOCAL_DURABLE_ACK, progress-only local
acks with both tiers requested, monotonic local watermarks, the
stray-frame warn path, watermark reset on reconnect, and the
replicated-ack-in-local-mode subsumption path.
- WebSocketResponseLocalDurableAckTest round-trips the
STATUS_LOCAL_DURABLE_ACK wire format through writeTo/readFrom,
isStructurallyValid, truncation rejection, and status naming.
- TestWebSocketServer now captures the X-QWP-Request-Durable-Ack
header and echoes the granted set (the legacy "true" request still
confirms as "enabled"), with a fixed-grant override for mismatch
tests. Integration tests drive local and local,replicated flows
through the real handshake, pin the exact request token per
configured set, assert denial when the server grants a different
set, and pin the boolean connect overload's legacy mapping.
- DurableAckIntegrationTest.awaitOks counted one OK per row, but the
server emits one OK per batch, so the helper always spun its full
5s deadline and returned silently. awaitOkBatches counts batches
and fails loudly on timeout; the suite drops from ~20s to under 1s.
Comment cleanup in the same area: the compat-overload block,
DurableAckTiers javadoc and Sender.requestDurableAck(boolean) now
describe what the code does instead of narrating the retyping and
release history ("was retyped", "shipped meaning").
Contributor
[PR Coverage check]😍 pass : 106 / 147 (72.11%) file detail
|
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
2 participants
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.
The sender can request durable acknowledgements by tier instead of a single
boolean.
request_durable_ackacceptson,off,local,replicatedandlocal,replicated.local: the server emitsSTATUS_LOCAL_DURABLE_ACK(0x0E) frames oncecommits are fdatasync-durable on its disk. The sender trims its
store-and-forward copy on them. Local durability survives power loss, not
the loss of the server's disk.
replicated: unchanged shipped behaviour — the server emitsSTATUS_DURABLE_ACKframes once commits reach the object store, and thesender trims on them.
local,replicated: both streams; the sender trims on the replicated ack(the strongest requested guarantee) and receives local acks as early
progress signals, exposed per table via
CursorWebSocketSendLoop.getLocalDurableTableWatermark.onkeeps its shipped meaning as the legacy alias for the replicated tier: itstill travels as the literal
trueheader value and expects the historicalenabledconfirmation, so nothing changes on the wire for existingconfigurations.
The grant is all-or-nothing. The server echoes the granted set verbatim (or
enabledfor a legacy request); a partial or missing echo counts as a denialand fails the connect with the existing mismatch error. This is a behaviour
change worth noting: requesting a tier the server cannot serve now fails
loudly at connect instead of silently proceeding without durable acks —
including explicit tokens sent to servers that predate them.
Binary compatibility is preserved: the nine public
QwpWebSocketSender.connectoverloads that took the boolean flag remain asdelegating overloads (verified by
ExportedApiCompatibilityTest), andSender.LineSenderBuilder.requestDurableAck(boolean)is unchanged. A newrequestDurableAck(CharSequence)overload accepts the tier tokens.Server support for the
localtier lands via questdb/questdb#7411 (branchnw_adaptive_commit), which carries end-to-end tests driving this client —including power-loss crash proofs that the local ack never covers a
transaction the server's disk does not hold. The Enterprise tandem of that
branch is questdb/questdb-enterprise#1147; composing the
localandreplicatedtiers on an Enterprise server is follow-up work there.Test plan:
routing (
CursorWebSocketSendLoopDurableAckTestand siblings)WsSenderConfigHonoredTestcovers the new config valuesExportedApiCompatibilityTestverifies the restored boolean overloadsquestdb/questdb branch