Skip to content

Native: answered queries no longer hold query queue capacity - #965

Merged
rmcdaniel merged 6 commits into
mainfrom
rust-964-query-queue-capacity
Nov 3, 2026
Merged

rmcdaniel merged 6 commits into
mainfrom
rust-964-query-queue-capacity

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Review tier A: the query claim scan feeds lease and fence decisions (write-path locking and concurrency). It's a narrow change: what the scan reads, plus an expiry sweep.

Fixes #964.

Cause

Native keeps every query task in dw_query_cache until its receipt expires (receipt_expires_at, the deadline plus at least 60 s), whatever its status. query_entries scanned the namespace's entries with LIMIT 65 regardless of status:

  • admission refused at 64 entries (MAX_ENTRIES);
  • query_entries itself refused above 64, and the worker claim scan and the task-queue admission view also call it.

So 64 answered queries in one namespace refused every new query (503 query_task_queue_full) and blocked claims until the oldest receipts expired. That matches Python SDK 2.12.4's cutoff at query 64 and the ~4–5 minute recovery. PHP 2.11.8's admission counts only pending task ids against max_pending_per_queue, and answered all 75.

Change (redesigned at 5f78e8f; see the PR comment)

  • Open query tasks are listed in one exact-key open-set row per namespace. Admission, claims and the queue view read that set and fetch each task by key; answered tasks hold no capacity.
  • Every query row is filed at admission under the minute it expires, in capped pages. A watermark sweep deletes expired keys by key, with a hard per-call bound.
  • A one-time global cutover pages through the cache table by primary key (64 rows per call) to adopt 3.0.1 rows.
  • No schema change and no table scans on the request path; receipts keep their keys.

Checked

  • New answered_queries_release_queue_capacity: 80 sequential queries on one run, each claimed, answered and returned before the next. It fails at query 64 on 923d45b3 ("query 64 was not claimed") and passes with this change.
  • Full cargo test --locked on SQLite passes (672 execution, 397 lib).
  • All 37 --test execution query scenarios pass on PostgreSQL 16.15, MySQL 8.0.46 and MariaDB 10.11.19 (the CI image digests).

Not in this PR

Native answers query_task_queue_full with 503, PHP with 429. General decided native should match 429. That's a separate small PR, after checking the SDKs' handling.

Ships in the next native patch with #953.

🤖 Generated with Claude Code

Native keeps every query task in dw_query_cache until its receipt expires
(deadline plus at least 60 s), whatever its status. query_entries scanned
the namespace with LIMIT 65 across all statuses, and admission refused at 64
entries. So 64 answered queries in a namespace refused every new query, and
blocked the worker claim scan, until the oldest receipts expired. Python SDK
2.12.4 hit it at query 64 of a sequential loop; PHP 2.11.8 answered all 75.

The scan now reads only open (pending or leased) entries on each backend,
and a bounded sweep removes expired receipts, which the scan used to do.
Answered, failed and timed-out receipts stay readable by key until they
expire. Admission keeps its 16-open-task bound, as PHP's counts only
pending task ids.

answered_queries_release_queue_capacity runs 80 sequential queries on one
run. It fails at query 64 on 923d45b and passes now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review at exact head 6594f94c9d6e740bf0ac5a599066763335efd2ba: release-blocking reliability / tenant-isolation finding.

The status transition itself fixes #964's accounting, but it removes the only total-receipt bound while both new database paths are unbounded scans:

  • QUERY_CACHE_SCAN now filters namespace and status out of the JSON value before LIMIT 65.
  • QUERY_RECEIPTS_EXPIRED filters expiration before LIMIT 32.
  • dw_query_cache has only its primary-key index on key; there is no namespace/status expression/generated-column index and no expiration index on SQLite, PostgreSQL, MySQL, or MariaDB.
  • Completed/failed/timed-out receipts remain for 180 seconds by default (and the TTL is configurable), and this head no longer caps how many terminal receipts a namespace can retain. Every admission, poll claim, and queue diagnostic calls query_entries.

The concrete SQLite plan for the exact query shapes is SCAN dw_query_cache USING INDEX sqlite_autoindex_dw_query_cache_1 for both. With 200,000 retained terminal rows before one victim open row, the open scan visits the retained set to return one row; the LIMIT does not bound database work. PostgreSQL's value::jsonb predicates and MySQL/MariaDB's JSON_EXTRACT likewise have no supporting index in the shipped catalogs. A high-query authorized namespace can therefore make another namespace's query admission/claim work proportional to the shared retained-receipt population; the global expiry sweep has the same shape. That is a cross-tenant availability regression on the path intended to unblock Cloud native.

Please preserve the terminal-receipt/idempotency contract while making both selection paths bounded by indexed state—for example, indexed materialized/generated namespace/status plus expiration columns, or a separately indexed/open-task representation—and add a high-terminal-cardinality counterfactual that proves admission/claim does not scan the retained population. The 80-query functional regression is necessary but does not exercise this resource bound.

I have not found an authority/scope error in terminal keyed reads, the 16-open admission cap remains enforced, and the multi-database status predicates themselves are consistent. Security hold is specifically the unbounded shared-table work above.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Design carry for the correction to Security's hold, before a new head is pushed:

The revised exact-key open-set design avoids the PostgreSQL collation trap in the first range-key proposal, but Security clearance will require these invariants in the exact implementation and tests:

  1. The per-namespace open-set value is bound to the full namespace (not trusted solely from a truncated hash), contains unique canonical task ULIDs, has a hard cardinality bound, and every exact-fetched task is revalidated for task id, namespace and open status. Stale, terminal and expired members are repaired transactionally.
  2. Terminal transition and indexing are atomic under the existing transition lock. Every terminal path is included: complete, fail, query timeout, invalid/missing deadline tombstone, and worker/lease retirement. Every keyed reader—including the original waiting request and idempotent completion retry—resolves the moved receipt without an open-key race.
  3. Expiry indexing is itself hard-bounded. A minute bucket cannot become an unbounded JSON row or hot rewrite: bucket by the actual receipt expiry and use capped exact-key pages (or an equivalent representation with explicit row/keys/work bounds). Watermark/page updates must be crash-atomic, and a TTL policy change cannot force an unbounded search.
  4. Missing per-namespace state must not trigger an unbounded legacy JSON/LIKE scan for every new or attacker-selected namespace. Existing 3.0.1 state needs an explicit global cutover/migration marker with bounded primary-key pagination, or equivalent evidence that fallback is one-time and cannot be amplified through namespace names.
  5. Counterfactual evidence includes thousands of retained receipts, corrupt/stale set members, interrupted expiry progress, old 3.0.1 open rows, and query admission/claim/status/idempotent completion on SQLite, PostgreSQL, MySQL and MariaDB. The database plans should show only bounded primary-key operations on the steady path.

These are acceptance constraints, not a request to widen the patch into a schema migration. The current exact head remains held by the unbounded scans in comment 6589640509 until its replacement is reviewed.

…ans (#964)

Security's hold on 6594f94: filtering the namespace scan to open rows left
it, and the expiry sweep, scanning every retained receipt in the shared
cache table (JSON predicates and expiration have no index). A key range
isn't safe either: on PostgreSQL the key column uses the database's
collation (en_US.utf8 in the CI image), which doesn't order punctuation
bytewise.

Every request-path read is now by primary key:
- Open set: one row per namespace (key from a namespace digest, value
  naming the namespace in full) lists its open task ids, validated as
  unique canonical ULIDs, at most 64. Admission adds the new id. Each read
  fetches the listed tasks by key, keeps only this namespace's pending or
  leased ones under their own id, and rewrites the set when members closed,
  expired or vanished.
- Expiry: every task row is filed at admission under the minute it expires,
  in pages of at most 128 keys. A sweep behind a watermark row deletes
  expired keys by key, at most 30 minutes and 2 pages per call, and files a
  key again if its expiry moved. A row already expired when filed is
  deleted at once. All of it runs in the caller's transaction.
- Cutover: 3.0.1 kept query rows with neither. A global marker walks the
  cache table one primary-key page (64 rows) per call, once for the whole
  server, listing open tasks in their namespaces' sets and filing every
  query row. No per-namespace fallback scan exists.

Receipts keep their keys, so the waiting caller, idempotent retries and
completions read them as before. thousands_of_retained_receipts_leave_
queries_unaffected runs 20 queries with 3000 answered receipts present.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Redesigned at 5f78e8fe after Security's hold (6589640509) and the follow-up constraints in chat. No schema change. Every request-path statement is a primary-key lookup.

Why not a key range: the key column on PostgreSQL uses the database's collation. The CI image's is en_US.utf8, which doesn't order punctuation bytewise, so key >= 'dw-native-query:' AND key < 'dw-native-query;' could miss rows.

Design

  • Open set: one row per namespace, keyed dw-native-query-open:<sha256(namespace)[..16 bytes]>, with a value that names the namespace in full.
    • Reads accept unique canonical ULIDs only, at most 64. A row naming another namespace lists nothing.
    • Each read fetches the listed tasks by key and keeps only this namespace's pending or leased tasks under their own id. It rewrites the set when a member closed, expired or vanished.
    • Admission adds the new id under the transition lock.
  • Expiry: every query row is filed at admission under the minute it expires (dw-native-query-expiry:<minute>, a header plus numbered pages of at most 128 keys).
    • A sweep behind a watermark row deletes due keys with DELETE … WHERE key=? AND expiration<=?: at most 30 minute headers and 2 pages per call.
    • A key whose expiry moved later is filed again; a row already expired when filed is deleted at once.
    • All of this runs in the caller's (serialized) transaction, so it's crash-atomic.
  • Cutover: 3.0.1 rows have neither. A global marker walks the whole cache table one primary-key page per call (key > ? ORDER BY key LIMIT 64), once per server, never per namespace. It lists open tasks in their namespaces' sets and files every query row.
  • Receipts keep their keys, so the waiting caller, idempotent poll and completion retries, and stale or duplicate completions read them unchanged. Terminal transitions need no bookkeeping beyond the open set's lazy repair, which counts only open members.

Evidence

  • thousands_of_retained_receipts_leave_queries_unaffected: 3000 answered receipts inserted as 3.0.1 left them, then 20 sequential queries all admitted, claimed and answered.
  • answered_queries_release_queue_capacity (80 sequential queries) still passes. It fails at query 64 on 923d45b3.
  • The full SQLite suite passes (673 execution, 397 lib). All 38 query scenarios pass on PostgreSQL 16.15 (en_US.utf8), MySQL 8.0.46 and MariaDB 10.11.19.
  • EXPLAIN on a 5000-row table:
    • SQLite: SEARCH … USING INDEX sqlite_autoindex_dw_query_cache_1 (key=?) for get, update and delete-expired; (key>?) for the cutover page.
    • PostgreSQL: Index Scan using dw_query_cache_pkey, with Index Cond: (key = …), and for the page (key > '') under Limit 64.
    • MySQL 8.0 / MariaDB 10.11: a const or PRIMARY range for get and delete; a PRIMARY range for the page.

The tier stays A. PR description updated.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review at exact 5f78e8f: the new admission/claim open-set path is bounded, but the shared-table scan still has two live poll paths, so this head remains held.

  • queries.rs::query_poll_ready executes DB::POLL_WAKE_QUERIES_SQL on every held query-poll readiness probe.
  • cross_kind_poll.rs::poll_wake_snapshot executes the same statement for every non-query poll whose worker opted into cross-kind wake.
  • On all three backends that statement is still a key-prefix predicate plus expiration and JSON namespace/queue/deadline/status predicates over dw_query_cache. The table has only its key PK for this access pattern. LIMIT 65 bounds returned matches, not query receipts inspected; retained receipts from other namespaces can therefore impose repeated work on another tenant's long polls.

This also makes the claim that steady-state query paths are exact-key-only false even though query_entries itself now is.

Please replace both uses with the namespace open-set row plus at most 64 exact task-key GETs (or another genuinely indexed/bounded authority), and remove/retire the shared JSON scan constants. Counterfactual evidence should exercise (1) a held query poll and (2) a cross-kind-enabled non-query poll behind thousands of retained receipts, proving those probes issue only the open-set GET plus bounded exact task GETs—not merely that a planner labels the prefix walk an index scan.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Additional atomicity requirement on exact 5f78e8f while the poll scan is being corrected: cut_over_query_rows uses let _ = Self::list_open(...).await. That discards every failure, although the comment only intends to tolerate query_task_queue_full. On a backend where a statement failure does not poison the transaction, a write/encoding/storage error can be swallowed and the cutover marker can still advance past that legacy open task permanently.

Please suppress only the explicitly intended full-set refusal (if dropping that task to its existing deadline remains the chosen migration behavior) and propagate every other error, so the task indexing, expiry filing, and page-marker advance remain one atomic step. A fault-injection counterfactual should leave the marker at the prior page when open-set persistence fails.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Concrete cutover correctness case at 5f78e8f: cut_over_query_rows calls file_expiry first, but when expiration <= now that helper deletes the row and returns no indication. Cutover then continues parsing the stale fetched value and can add its pending/leased ID to the namespace open set.

This can permanently skip a live legacy task across tenants: place 64 expired rows whose stored status is still pending for namespace A before one unexpired open A row in key order; drive every cutover page from namespace B. The first page deletes the 64 rows but fills A's set with their IDs. B repairs only B's set. The later live A row then gets the intentionally swallowed queue-full refusal, the global marker advances past it, and only afterwards can A repair its stale set—too late to rediscover the live task.

Expired rows must be deleted and excluded from open-set indexing in the same page transaction. Please add that two-namespace counterfactual and prove the live A task is claimable after B alone drives cutover to done.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The same cross-tenant case applies before physical cache expiration: a legacy row can retain status=pending/leased after its query deadline while its receipt expiration is still in the future. Cutover currently trusts the stored status without authority::expire, so 64 deadline-expired A rows can fill A's set and make the later live A row get skipped while B drives the cursor.

The page therefore needs to apply the query authority transition before deciding whether a legacy row consumes an open-set slot (persisting the terminal form as appropriate), not only test the cache expiration. Extend the two-namespace counterfactual to cover both physically expired rows and deadline-expired-but-retained rows.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Severity correction after checking the exact 3.0.1 admission invariants: normal old state capped open pending/leased tasks at 16 and all retained query rows for a namespace at 64. Therefore the 64 stale-open saturation sequence in my preceding comments cannot arise from valid 3.0.1 traffic alone; it requires corrupted/out-of-contract cache state (or a broken prior invariant).

Please still avoid indexing a row that the same cutover just deleted, apply authority expiry before classifying it open, and do not swallow unrelated persistence failures—those keep the migration internally consistent and are cheap counterfactuals. But I am not treating that saturation scenario as a second normal-upgrade release blocker. The demonstrated blocker at this head remains the two live repeated POLL_WAKE_QUERIES_SQL paths over retained cross-tenant receipts.

Security's re-review of 5f78e8f:
- query_poll_ready and poll_wake_snapshot still ran POLL_WAKE_QUERIES_SQL,
  the shared-table prefix plus JSON scan, on every long-poll probe. Both
  now use ready_query_keys: the namespace's open set read lock-free by key,
  then each listed task by key, with the old filters (queue, query deadline,
  pending or lease-expired). The statement is removed from every backend.
- The cutover deletes an already-expired row and never lists it, closes a
  pending row past its query deadline before it could take a slot, and
  propagates every failure except a full set, so its page and marker roll
  back together.

the_cutover_lists_only_answerable_rows: 128 stale 3.0.1 rows (expired, or
pending past their deadline) leave three live queries answered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review of authoritative head 5544ba68bb75ae9ec26880126005d8160aefc3eb: the demonstrated #964 blocker is corrected in source.

I verified the exact three-commit diff from base and the final four-file delta from 5f78e8fe. Both repeated live poll paths are now keyed: query_poll_ready and cross-kind poll_wake_snapshot call ready_query_keys, which reads one namespace-bound open-set row and at most 64 canonical unique task IDs, then exact primary-key rows. Before returning a hint it rebinds the stored task to its key ID and namespace, checks exact queue, physical expiry, query deadline, pending or lease-expired status, and never grants claim authority. Cross-kind preserves the prior query:<cache-key> snapshot identity. POLL_WAKE_QUERIES_SQL is removed from all backends and the MySQL statement allowlist; there are zero references.

The cutover hardening is also present: a physically expired row is deleted and not listed; authority expiry runs before open classification; only query_task_queue_full is deliberately tolerated; every other error aborts the page transaction before marker advance. The added stale-row regression covers both physical expiry and deadline expiry alongside live claims. As previously corrected, that 128-row saturation is defensive corrupted-state coverage rather than a valid 3.0.1 traffic sequence.

The old QUERY_CACHE_SCAN backend constants remain dead (no call sites); that is cleanup, not a live release hold. A malformed stored leased task without lease_expires_at could produce a readiness hint, but native-created leased rows carry that authority field and the locked claim remains decisive, so I am not elevating corrupted-row behavior into this release gate.

Source clearance: exact 5544ba68… resolves Security’s query-wedge finding. This is not yet merge clearance: the PR is BEHIND current base and hosted Rust/contracts/PHP checks are still running. Please integrate current main, preserve this source, and require the fresh hosted set green before merge.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security hosted-gate update for exact head cd1caa69d104c25b9882c0edee0757ff7041885b on current protected main 1124d86a45af0b824b523d18292e22308767fd9f:

  • GitHub's tested synthetic merge is 7a718959c14f27edb09e10009875573b18ec4b35, with parents exactly 1124d86a… + cd1caa69….
  • The unchanged-head rerun cleared the unrelated WorkflowStreamPayloadRaceTest failure. PHPUnit feature suite and the dependent Feature source qualification both pass.
  • The full exact-head rollup is now terminal: 57 success + 1 intentional candidate-source skip, with zero failures or pending checks. The PR is OPEN and CLEAN.
  • The feature source remains the Security-cleared query-open-set/poll-wake correction recorded in 6590183396; the current-base carries did not alter that scope.

This establishes a clean current-base result, but it is deliberately not final merge clearance yet because Security-cleared #957 must merge first and advance main. After #957 merges, carry that new protected main into #965, preserve the reviewed query delta, and require the resulting hosted set terminal green. Do not merge this soon-to-be-stale base ahead of #957.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security post-#957 carry review at exact bae9138: its parents are the previously reviewed cd1caa6 and exact merged main 9524e80; the merge has no combined-diff conflict resolution. The reviewed PR delta is content-equivalent across the carry (stable patch ID b6af62836678681be5bf0c42995d90b111da472e before and after), and the five release/identity files changed by #957 do not overlap the five query-runtime/test files changed here. Source carry is clear; final merge clearance still waits terminal hosted checks on this exact head.

@sonarqubecloud

sonarqubecloud Bot commented Nov 2, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final carry clearance: exact head bae91380daceede64454c684cdaf67237795e77d is terminal.

Security final-clear is granted for #965 at bae91380…. Preserve this exact head through the merge queue; any content change needs a fresh carry. This does not itself authorize a release dispatch.

@rmcdaniel
rmcdaniel added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 6ea2d05 Nov 3, 2026
58 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Post-queue Security receipt: merged as 6ea2d051c68c8f30e3f76015ecbc1de28be6440d. Its parents are protected main 9524e80b2f88e6282aacbcd51560a080302d2d33 and the exact final-cleared head bae91380daceede64454c684cdaf67237795e77d; merge tree 82f066524d1da7f21c660560cfcf877056df7907 equals the reviewed head tree. The customer query-capacity wedge fix is now on main. Publication remains gated on the rest of the 3.0.2 composition and release acceptance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native query_task_queue_full after ~63 sequential queries; completed query tasks appear to keep consuming capacity

1 participant