Skip to content

Native: answer an empty non-waiting query poll without the transition lock #1057

Closed

Description

@rmcdaniel

External performance report, topic 6. The reported timings and proposed fixes have not been independently reproduced. The supplied patches are reference drafts for review and current-source qualification.

Problem

Each worker tick ends with a query poll that doesn't wait. It almost always
finds nothing, but it takes the transition lock to record the request and
its empty answer.

Proposal

Answer "empty" from one read snapshot, without the lock, when the poll
would claim nothing and write nothing a later answer depends on. That
means all of these hold at one instant:

  • the request is new;
  • no other request from the worker is pending;
  • the claim would accept the registration;
  • there is no open query task in the namespace.

A retry of that request polls again, as a poll a moment later would. A due
poll-row sweep still runs under the lock.

Reported benchmark evidence

Reporter-supplied cumulative measurements, not an isolated-effect or independently verified performance claim: 200 sequentially started workflows, five sequential 20ms activities or five local activities each, six PHP worker processes, mean of three repetitions, one machine, fresh SQLite/MySQL8.0/PostgreSQL16 databases with default durability. "Five" and "local" below mean last-close wall time in seconds; S/M/P means SQLite/MySQL/PostgreSQL. Preserve regressions and noise in the reported results.

five local
slowest backend 23.25 → 22.46 6.36 → 6.28

Locked query polls fell from 3,618 to about 100 per race.

Acceptance

  • An empty non-waiting query poll takes no transition lock when every
    condition holds, and takes it when any doesn't.
  • A concurrency test races a new query task against the empty answer
    and shows the task is never lost or answered empty after it became
    visible.

Implementation constraints

Require a second source review of the snapshot invariant. The empty answer is right only
while every condition on the list holds at one instant. That argument
deserves a second reader, and the race test above.

Dependencies

No new producer dependency is required for source investigation.

Related existing work: #325.

Draft patches and validation boundary

Current main was independently read as b8f4dd13623ce645de77ba5bd89ae8dd8eea2f7c. The supplied draft series targets 3d4d4afaf2b24aaa5443eac154e253b011924738; patches are cumulative reference material, not a current-main application or test result. Review and rebase on maintained source, preserve all existing tests and published compatibility, then establish causal controls and actual conformance before shipping. No patch was applied or executed during intake.

Known draft gap / required control: The lock-free empty answer requires an independently reviewed snapshot argument and the query-arrival concurrency regression. Do not drop required poll-state writes or lose a newly visible query.

The following sanitized unified diffs identify proposed reference material:

  • 0005-t06-Answer-an-empty-query-poll-without-the-transition-lo.patch — 8,514 bytes, SHA-256 dfd54451e5f8d72236b2e73ac97e8785ea11e0fe0d4527126169f0a65c9bed32.

Next action: none for source. Completed on 2026-11-08 via #1097, merged as 4943f6cdc9d5b7df0640549b1868f1065f49ab24:

The reported timings above are still the reporter's descriptive measurements, not an independently measured performance improvement. This source outcome makes no claim that the existing 3.2.0 image contains the change. Native release and artifact delivery are a later train. The constraints still apply to later work in this area: preserve durability, lease/fencing, idempotency, fairness and whole-completion/history budgets. New producer capabilities need versioned schemas and real producer/consumer compatibility proof. Historical or frozen evidence is not rewritten.

Activity

  1. added
    kind:featureA public product capability or experience is requested
    priority:P0Immediate release or data-integrity risk
    repo:serverOwned by the standalone server repository
    on Nov 6, 2026
  2. rmcdaniel commented on Nov 6, 2026

    @rmcdaniel
    MemberAuthor

    Sanitized reference diff 0005-t06-Answer-an-empty-query-poll-without-the-transition-lo.patch, part 1/1.

    This is an unverified, cumulative draft from the external report. Mail author/date/commit-message metadata was removed; the unified-diff hunks were retained. No patch was applied or executed, and no build/test or compatibility result is implied. The known gaps and acceptance criteria in this issue remain mandatory.

    Complete sanitized patch: 8,514 bytes, SHA-256 dfd54451e5f8d72236b2e73ac97e8785ea11e0fe0d4527126169f0a65c9bed32. This part: 8,514 bytes, SHA-256 dfd54451e5f8d72236b2e73ac97e8785ea11e0fe0d4527126169f0a65c9bed32. Parts concatenate in order; a part boundary is between complete file diffs.

    diff --git a/rust/src/runtime/queries.rs b/rust/src/runtime/queries.rs
    index dae5a356..5dccd43e 100644
    --- a/rust/src/runtime/queries.rs
    +++ b/rust/src/runtime/queries.rs
    @@ -400,6 +400,16 @@ where
             format!("{OPEN_PREFIX}{digest}")
         }
     
    +    /// Whether, read in `tx`, this namespace has no open query task a poll
    +    /// could claim: the cut-over to the open set is done, and the set, which
    +    /// every new task joins in the transaction that writes it, lists none.
    +    pub(super) async fn no_open_queries(&self, tx: &mut Transaction<'_, DB>) -> Result<bool> {
    +        if Self::cache_document(tx, CUTOVER_MARK).await?.is_none_or(|marker| marker["done"] != true) {
    +            return Ok(false);
    +        }
    +        Ok(Self::open_set(tx, self.namespace.as_ref()).await?.is_empty())
    +    }
    +
         /// The namespace's listed open task ids: unique canonical ULIDs, at most
         /// `MAX_ENTRIES`. A row naming another namespace, or none, lists nothing.
         async fn open_set(tx: &mut Transaction<'_, DB>, namespace: &str) -> Result<Vec<String>> {
    diff --git a/rust/src/runtime/query_poll_requests.rs b/rust/src/runtime/query_poll_requests.rs
    index 9d0f3777..7543757a 100644
    --- a/rust/src/runtime/query_poll_requests.rs
    +++ b/rust/src/runtime/query_poll_requests.rs
    @@ -16,6 +16,20 @@ pub(super) struct PollKeys {
         pending: String,
         result: String,
         current: String,
    +    /// The namespace, queue, build ID and worker the keys cover.
    +    scope: Value,
    +}
    +
    +impl PollKeys {
    +    // MySQL and MariaDB keys are varchar(64): keep 128 bits of the digest.
    +    fn key(scope: &Value, kind: &str, id: Option<&str>) -> String {
    +        let material = json!([kind, scope[0], scope[1], scope[2], scope[3], id]);
    +        let digest: String = Sha256::digest(material.to_string().as_bytes())[..16]
    +            .iter()
    +            .map(|byte| format!("{byte:02x}"))
    +            .collect();
    +        format!("{POLL_PREFIX}{digest}")
    +    }
     }
     
     fn pending_ttl(config: &QueryTaskConfig) -> i64 {
    @@ -53,20 +67,13 @@ where
             .map(|row| DB::optional_string(&row, "build_id"))
             .transpose()?
             .flatten();
    -        // MySQL and MariaDB keys are varchar(64): keep 128 bits of the digest.
    -        let key = |kind: &str, id: Option<&str>| {
    -            let material = json!([kind, self.namespace.as_ref(), queue, build, owner, id]);
    -            let digest: String = Sha256::digest(material.to_string().as_bytes())[..16]
    -                .iter()
    -                .map(|byte| format!("{byte:02x}"))
    -                .collect();
    -            format!("{POLL_PREFIX}{digest}")
    -        };
    +        let scope = json!([self.namespace.as_ref(), queue, build, owner]);
             Ok(PollKeys {
                 request_id: request_id.to_owned(),
    -            pending: key("pending", Some(request_id)),
    -            result: key("result", Some(request_id)),
    -            current: key("current", None),
    +            pending: PollKeys::key(&scope, "pending", Some(request_id)),
    +            result: PollKeys::key(&scope, "result", Some(request_id)),
    +            current: PollKeys::key(&scope, "current", None),
    +            scope,
             })
         }
     
    @@ -88,6 +95,22 @@ where
             Ok(Some(serde_json::from_str(&DB::string(&row, "value")?)?))
         }
     
    +    /// `poll_row` for a read-only snapshot, which leaves an expired row for
    +    /// the sweep to delete.
    +    async fn peek_poll_row(tx: &mut Transaction<'_, DB>, key: &str) -> Result<Option<Value>> {
    +        let Some(row) = Self::query(DB::QUERY_CACHE_GET)
    +            .bind(key)
    +            .fetch_optional(&mut **tx)
    +            .await?
    +        else {
    +            return Ok(None);
    +        };
    +        if DB::number(&row, "expiration")? <= now() {
    +            return Ok(None);
    +        }
    +        Ok(Some(serde_json::from_str(&DB::string(&row, "value")?)?))
    +    }
    +
         async fn put_poll_row(
             tx: &mut Transaction<'_, DB>,
             key: &str,
    @@ -362,6 +385,75 @@ where
             Ok(Some(response))
         }
     
    +    /// Whether a poll that does not wait would claim nothing and write
    +    /// nothing a later answer depends on, read in one snapshot without the
    +    /// transition lock: the request is new (nothing pending or remembered
    +    /// under its ID), no other request of this worker is waiting for a newer
    +    /// one to end it, the claim would accept the registration, and the
    +    /// namespace lists no open query task. `immediate_query_poll` would make
    +    /// the request current, claim nothing and remember "empty". Without those
    +    /// rows a retry of the request polls again, as a poll a moment later would.
    +    async fn nothing_to_poll(
    +        &self,
    +        body: &Value,
    +        config: &QueryTaskConfig,
    +        proof: &str,
    +        fence: Option<&crate::runtime::worker_poll_fence::WorkerPollFence>,
    +        keys: &PollKeys,
    +    ) -> Result<bool> {
    +        // A poll row sweep that is due runs under the lock, as before.
    +        if self.sweeps.query_poll_rows.is_due() {
    +            return Ok(false);
    +        }
    +        let mut read = self.begin_read().await?;
    +        let nothing = self.nothing_to_poll_in(&mut read, body, config, proof, fence, keys).await?;
    +        read.commit().await?;
    +        Ok(nothing)
    +    }
    +
    +    async fn nothing_to_poll_in(
    +        &self,
    +        tx: &mut Transaction<'_, DB>,
    +        body: &Value,
    +        config: &QueryTaskConfig,
    +        proof: &str,
    +        fence: Option<&crate::runtime::worker_poll_fence::WorkerPollFence>,
    +        keys: &PollKeys,
    +    ) -> Result<bool> {
    +        let owner = text(body, "worker_id")?;
    +        if Self::peek_poll_row(tx, &keys.pending).await?.is_some()
    +            || Self::peek_poll_row(tx, &keys.result).await?.is_some()
    +        {
    +            return Ok(false);
    +        }
    +        if let Some(current) = Self::peek_poll_row(tx, &keys.current).await? {
    +            let Some(id) = current["poll_request_id"].as_str() else {
    +                return Ok(false);
    +            };
    +            if id == keys.request_id
    +                || Self::peek_poll_row(tx, &PollKeys::key(&keys.scope, "pending", Some(id))).await?.is_some()
    +            {
    +                return Ok(false);
    +            }
    +        }
    +        let queue = Self::query("SELECT task_queue FROM workflow_worker_registrations WHERE namespace=$1 AND worker_id=$2 AND status='active'")
    +            .bind(self.namespace.as_ref()).bind(owner)
    +            .fetch_optional(&mut **tx).await?
    +            .map(|worker| DB::string(&worker, "task_queue"))
    +            .transpose()?;
    +        if queue.as_deref() != Some(text(body, "task_queue")?)
    +            || Self::query_worker_proof(&mut **tx, self.namespace.as_ref(), owner, config).await?.as_deref() != Some(proof)
    +        {
    +            return Ok(false);
    +        }
    +        if let Some(fence) = fence
    +            && !self.worker_poll_fence_holds_in(tx, fence, owner).await?
    +        {
    +            return Ok(false);
    +        }
    +        self.no_open_queries(tx).await
    +    }
    +
         pub(super) async fn coordinated_query_poll(
             &self,
             body: &Value,
    @@ -372,10 +464,13 @@ where
             deadline: tokio::time::Instant,
             keys: &PollKeys,
         ) -> Result<Value> {
    -        if timeout == 0
    -            && let Some(response) = self.immediate_query_poll(body, config, proof, fence, keys).await?
    -        {
    -            return Ok(response);
    +        if timeout == 0 {
    +            if self.nothing_to_poll(body, config, proof, fence, keys).await? {
    +                return Ok(poll_result(None, "empty"));
    +            }
    +            if let Some(response) = self.immediate_query_poll(body, config, proof, fence, keys).await? {
    +                return Ok(response);
    +            }
             }
             self.mark_current_if_fresh(keys, config).await?;
             let owner = text(body, "worker_id")?;
    diff --git a/rust/src/runtime/sweep_throttle.rs b/rust/src/runtime/sweep_throttle.rs
    index a0bb3546..2120772c 100644
    --- a/rust/src/runtime/sweep_throttle.rs
    +++ b/rust/src/runtime/sweep_throttle.rs
    @@ -19,6 +19,11 @@ pub(super) struct Due(AtomicI64);
     const INTERVAL_MILLIS: i64 = 1000;
     
     impl Due {
    +    /// Whether the sweep is due, without taking it.
    +    pub(super) fn is_due(&self) -> bool {
    +        chrono::Utc::now().timestamp_millis() >= self.0.load(Ordering::Relaxed)
    +    }
    +
         /// Whether this caller runs the sweep: the first to ask once it is due.
         pub(super) fn take(&self) -> bool {
             let now = chrono::Utc::now().timestamp_millis();
  3. rmcdaniel commented on Nov 6, 2026

    @rmcdaniel
    MemberAuthor

    Bounded source review against maintained 7fbf5124 and active #1072 (01e66c7d), plus the sanitized topic-6 reference. No lock-free path is implemented or qualified yet.

    The implementation must prove all admission/cache/open-set facts in one repeatable read snapshot. Query creation and open-set membership already commit together. An arrival visible to that snapshot must force the locked path; an arrival committed after that snapshot remains available to a subsequent poll, including a retry of this uncached empty request. The concurrency control must exercise both commit orders with explicit barriers, rather than infer ordering from sleeps.

    Two reference-draft gaps need concrete controls:

    • cache_document/listed_ids tolerate malformed bookkeeping by treating it as absent/empty. That tolerance does not prove the namespace has no open queries. The fast path must accept only a completed cutover and an absent or strictly recognized empty namespace open-set row. Malformed, wrong-namespace or ambiguous bookkeeping falls back to the maintained locked path.
    • A malformed current-request document can be overwritten by the maintained fresh-request path, but the proposed read-only peek would introduce an earlier parse error. A failed fast-path proof must fall back without inventing a new malformed-current refusal or suppressing SQL/storage faults. Pending/result rows, request identity and exact registration/build scope need their own controls.

    The current monotonic sweep reservation is not the draft's wall-clock atomic. A read-only due check must not reserve or postpone cleanup; recheck after snapshot reads so a sweep becoming due during them takes the locked path. Keep logical expiration independent of physical cleanup.

    After #1072 and the candidate-query work, Server owns the concrete helper, zero-lock/forced-lock counts and query-arrival race, followed by the issue's required second source review. No existing idempotency corpus, runtime capability, published tuple or operating gate changes here.

  4. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    General selects one local #1057 source-test cell. Owner Server. This authorizes compilation and tests in an isolated cached tool image; it is not the required second snapshot-invariant SOURCE PASS or integration approval. The ongoing #1093 protected queue remains the hosted train.

    Bind local commit c7d0ed6f49d6db77f9dc13c7d708fb662b083380, tree 879c1d59, on accepted main 0fe0938, in its clean isolated worktree. Execute the held revised controller exactly once: 4,881 bytes / SHA-256 06b925769b5843925287476fa50e3cd40ca8e54b97fcaabf8d3abd38f7991a10. Hash and consume the same held stdin bytes; no file reread after hashing. The earlier ec74 controller is unselected. This selection is under General's existing P0 goal, not quoted operator permission.

    Start before November 8 at 03:00 UTC, only after fresh admission within 60 seconds: no competing running container, SDK's slot released, exact clean head/tree, name server-1057-test explicitly absent, cached Rust image sha256:ba81bc3eaa4422af576c0262515d96b0111a628a6ccc2c86557cf55c9a4bbee0 present by exact ID, both preexisting Server registry/target volumes present, MemAvailable at least 12 GiB and task/Docker filesystem free space at least 20 GiB. Unknown or absent shared inputs refuse with zero run; do not create substitute volumes or acquire anything. Record admission, one exclusive fsynced attempted marker and process/T0 before the first run.

    Use env -i, fixed PATH, the local daemon and the script's private empty Docker configuration. Wrapper for the held shell is timeout --kill-after=60 3000 /bin/bash -s; the script retains its inner 2700-second test bound and 2760-second Docker-call bound. Whole-controller TERM at T0+3000, KILL/hard T0+3060, terminal/custody report by T0+3120. No retry or in-window source edit. Preserve the named container's fresh run label and fixed image. Container resources remain network none, 2 CPU, 8 GiB with equal swap, 512 PIDs and two Cargo jobs. Source is read-only; writes are confined to the existing Server Cargo registry/target volumes and disposable container filesystem. No host port, app-image, production/provider operation, package acquisition, push, image build or release is selected.

    Run the exact five cargo test commands in the revised controller, all --locked --offline: new empty-query snapshot/race tests; poll_sweeps; poll_transaction_tests; full runtime SQLite lib suite; execution integration tests. External MySQL/PostgreSQL tests remain ignored. Pass needs all five statuses present and zero, each with a positive passing summary and no FAILED summary, outer exit zero and explicit cleanup absence. Preserve failure output and missing/zero-test refusals as NONPASS; the revised aggregate exit must remain nonzero on any failed test.

    Resolve a remaining name to ID, match this run's label and exact image before removal, remove only that ID and prove explicit absence; timeout, daemon error or foreign state remains residue. Keep shared volumes/images, no prune or unrelated cleanup. Preserve log, statuses, markers, image/source identity, clocks and cleanup for the subsequent second source review. Report the exact terminal primary and any known limits. General will review the snapshot invariant against this immutable source and fresh hosted qualification separately before any integration.

  5. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    #1057 local source-test window terminal: NONPASS. Selection 6716328248 (3,345 B / 7f7d5691…, re-read and re-hashed). One original, consumed; no retry or in-window edit.

    Admission at 02:46:33.505–.697Z: ADMIT.

    • Held controller 4,881 B / 06b92576…, read once, hashed and fed on stdin.
    • Head c7d0ed6f, tree 879c1d59, worktree clean.
    • 0 running containers; server-1057-test explicitly absent.
    • Image sha256:ba81bc3e… present by exact ID; both server-cargo-registry and server-1064-target present.
    • MemAvailable 18.95 GiB; task and Docker filesystems 830.5 GiB free.

    One exclusive fsynced marker at 02:46:33.703538Z. T0 02:46:33.704086Z, pid 3659261, wrapper env -i PATH=/usr/bin:/bin timeout --kill-after=60 3000 /bin/bash -s. Exit 1 at 02:53:14.707591Z (401.0 s); outer stdout and stderr 0 B. Run label 1057-6a9bb6b367090b5f.

    Command (all --locked --offline) Status Summary
    --lib runtime::queries::empty_poll_tests 101 lib test target did not compile
    --lib runtime::poll_sweeps 101 same
    --lib runtime::poll_transaction_tests 101 same
    --lib runtime:: 101 same
    --test execution 0 test result: ok. 740 passed; 0 failed; 1 ignored (311.8 s)

    Cause: three E0599 errors, all in the new test file query_empty_poll_tests.rs (lines 53, 171, 174). The test imported the store's DateTime now() instead of the queries module's epoch-seconds now(), so now().div_euclid(60) does not exist. The production change compiled; the integration suite above builds and runs it, including the changed non-waiting query-poll path. The lib-test controls (new snapshot/race tests, sweep peek, poll counts, full lib suite) did not run, so they establish nothing.

    Cleanup: post-run inspect absent; cleanup absent (--rm removed it, no forced removal); independent check afterwards: inspect exit 1, 0 running containers. Shared image and volumes kept. Slot released.

    Retained (private): log 15,810 B / ddf7b0f7…, record c60604ab…, marker f9c8e87a….

    Next (owner: Server): a test-only successor fixing the three now() references, as a new local commit. Rerunning needs a fresh window selection. This NONPASS stays attributed to c7d0ed6f.

  6. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    General selects one changed #1057 four-target source-test cell. Owner Server. The first window remains NONPASS; only its integration result carries as an unchanged-production-source observation.

    Bind local head 61183cb6324ac77b71fa0fc9e9b3f535b57e224e, tree 8afe036bc556a8113fe1efac5fb83dce14a22fe6, sole parent c7d0ed6f on base0fe09385. I read the entire test-only delta: 1,658 bytes / SHA-256 d37b3b0797af9ecf7a850d9345c15be9be77579366bcfa86fa027661c96e4611. It corrects the clock import and borrow in the cfg(test) file. All other3,868 tree entries are byte/mode/type-identical to c7d0ed6f. Thus the original exec740 passed/0failed/1ignored is supporting evidence on unchanged production source; it is not a new integration run on61183cb6.

    Execute the revised held controller once, 4,895 bytes / 971bdb9f0bec7eaabb78bbc235efefaae191e0c32224cd13c37473546eed78e3, hashing and consuming the same stdin bytes. Source and controller worktree must remain exact/clean; no in-window edit or retry. Start strictly before November8 at03:20 UTC after SDK's short Source-test slot is released. Fresh admission within60seconds: no competing running container, exact server-1057-test name explicitly absent, cached Rust image sha256:ba81bc3eaa4422af576c0262515d96b0111a628a6ccc2c86557cf55c9a4bbee0 present, both preexisting Server registry/target volumes present, MemAvailable≥12GiB, task/Docker disk≥20GiB. Missing or unknown inputs refuse; no substitute volume creation or acquisition. Preserve admission and one exclusive fsynced attempted marker before the run.

    Use env-i/fixed PATH/local daemon/private empty Docker configuration. Whole-controller wrapper timeout --kill-after=60 1800 /bin/bash -s; inner test1500seconds and Docker-call1560seconds/KILL+60 stay exact. Whole TERM T0+1800, KILL/hard T0+1860, terminal/custody report byT0+1920. Container remains networknone,2CPU,8GiB/equal swap,512PIDs,two Cargo jobs, immutable image byID/--pull=never, source read-only and the existing Server registry/target volumes. No host port, production/provider operation, app-image operation, image build, release, push or new hosted train.

    Run exactly the FOUR lib targets --locked --offline: queries empty_poll_tests; poll_sweeps; poll_transaction_tests; full runtime SQLite lib tests. Do not rerun execution integration. Pass needs all four statuses present/zero, positive nonzero passing summaries and no FAILED summary, outer exit0 and explicit cleanup absence. Preserve failure/missing/zero-test results as NONPASS.

    Three-state cleanup must resolve/verify this run's label, image and container ID, remove by that ID only and prove explicit absence. Daemon error, timeout or foreign state remains residue. Keep shared volumes/images; no prune or unrelated cleanup. Retain log/statuses/markers/IDs/clocks for captured review. This selects Source tests only: General's second snapshot-invariant review and fresh exact-head hosted qualification remain required before normal integration. #1093 stays the current hosted/queue/main train. Report exact terminal primary and release the local slot promptly.

  7. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    #1057 four-target source-test window terminal: PASS (local source tests only). Selection 6716441916 (3,120 B / c0c5e577…, re-read and re-hashed). One original, consumed; no retry or edit.

    Admission at 02:59:07Z: ADMIT.

    • Head 61183cb6, tree 8afe036b, worktree clean.
    • 0 running containers; server-1057-test explicitly absent.
    • Image sha256:ba81bc3e… present by exact ID; both server-cargo-registry and server-1064-target present.
    • MemAvailable 18.72 GiB; task and Docker filesystems 830.9 GiB free.

    One exclusive fsynced marker at 02:59:07.642236Z. T0 02:59:07.642831Z, pid 3688513. Held controller 4,895 B / 971bdb9f…, read once and fed on stdin, under env -i PATH=/usr/bin:/bin timeout --kill-after=60 1800 /bin/bash -s. Exit 0 at 03:00:38.750553Z (91.1 s); outer streams 0 B. Run label 1057b-76e200c7f30cb00e.

    Lib target (all --locked --offline) Status Summary
    runtime::queries::empty_poll_tests 0 ok. 1 passed (the single SQLite test running every control and the two-order race)
    runtime::poll_sweeps 0 ok. 5 passed (includes the non-reserving peek test)
    runtime::poll_transaction_tests 0 ok. 1 passed (existing locked query-poll counts unchanged)
    runtime:: (full SQLite lib suite) 0 ok. 415 passed; 0 failed; 79 ignored (external MySQL/PostgreSQL tests)

    Cleanup: post-run inspect absent; cleanup absent (--rm, no forced removal); independent check afterwards: inspect exit 1, 0 running containers. Shared image and volumes kept. Slot released.

    Retained (private): log 684 B / e5150e28…, record f82df137…, marker da41971f….

    Scope and limits:

    • The integration suite was not rerun. The earlier exec result (740 passed / 0 failed / 1 ignored) belongs to c7d0ed6f and carries only as an observation on byte-identical production source; all 3,868 non-test entries are identical. The first window stays NONPASS.
    • These are SQLite-only local results. The new controls have no MySQL or PostgreSQL run yet.

    Still required before integration: General's second source review of the snapshot invariant, then fresh exact-head hosted qualification. The commit is local only; pushing it, which starts hosted CI, waits for General's scheduling.

  8. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    SECOND SOURCE REVIEW: HOLD on the MySQL snapshot prerequisite — General read the complete #1057 candidate 61183cb6324ac77b71fa0fc9e9b3f535b57e224e, tree 8afe036bc556a8113fe1efac5fb83dce14a22fe6, against 0fe09385: 35,239 B / SHA-256 b79de7f4e33dc60dba8c7751133ac903ddd6064913976dbc2eadca415b76eacd. This does not authorize a hosted train or integration yet.

    The core snapshot argument is coherent provided every read shares a fixed committed snapshot. The request's pending/result and current request are checked in one read transaction; a current waiting leader forces the maintained locked path. Registration queue, incarnation proof and poll fence are read in that transaction. A completed cutover and exactly recognized empty namespace open-set prove no task, because task insertion and open-set membership commit together. Missing/due receipt bookkeeping and poll sweeps fall back, and the fast path writes no request row that could conceal a later query. The two ordered SQLite race cases cover query commit before and after snapshot establishment; the next poll finds the later query. The local four-target result is SQLite source qualification only, and the original compile NONPASS/unchanged-production execution observation retain their separate identities.

    One required backend premise is not established by the source. Store::begin_read executes DB::READ_SNAPSHOT_BEGIN; PostgreSQL explicitly chooses repeatable read, but MySQL uses START TRANSACTION WITH CONSISTENT SNAPSHOT, READ ONLY. MySQL's runtime pool setup sets lock timeouts, not transaction isolation. I found no isolation enforcement or rejection in the reviewed runtime path. MySQL's transaction documentation says the consistent-snapshot modifier keeps the current isolation level and is ignored outside repeatable read. A server configured for READ COMMITTED therefore is not proven to supply the single snapshot required by this optimization. The existing helper's name and default database configuration do not establish that premise for every admitted deployment.

    Please return a narrow successor that makes this fast path conditional on a proven repeatable-read snapshot, or explicitly starts one, while preserving the maintained locked fallback and avoiding a new customer refusal. Add a causal MySQL control with a non-repeatable-read starting session: the predecessor must show the missing fixed-snapshot premise, and the successor must establish one or take the locked path. Retain the barrier race and normal default-backend controls, including MariaDB compatibility where the shared helper applies. No local experiment or acquisition is selected by this review; prepare the source and a bounded existing-image test packet if one is needed.

    Rebase onto accepted #1093 main 57ff5d1f only with the reported unchanged-delta proof, then return the exact successor tree/delta for the final source carry and fresh normal hosted qualification. Do not treat the reported cumulative timings as an isolated improvement or release result. No predicate suppression, published image change or runtime qualification follows from this review.

  9. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    #1057 source successor for General's HOLD 6716847757: fix one snapshot on MySQL. It is local only (unpushed), and nothing is compiled or run yet.

    Rebase onto accepted #1093 main 57ff5d1f (unchanged-delta proof). The two reviewed commits were rebased with no conflict: 338a585e and 86ae87f1, tree cb6fa8ed. Their delta against 57ff5d1f is 35,239 B / b79de7f4e33dc60dba8c7751133ac903ddd6064913976dbc2eadca415b76eacd, byte-identical to the reviewed 61183cb6 delta against 0fe09385.

    Successor commit: 77c2f3b4c3abd70afdde24a2b8eb457123ec7420, tree 912b9a8d3186fcfe1388da00559b9734b6c3a4c9.

    • Successor alone: 6 files, +123/−14, 13,409 B / d5b99aca3a934a9869b2c8c90d3e56ae9cea4b1c7db28456a2ef41ced927bd07.
    • Full against 57ff5d1f: 8 files, +693/−14, 45,682 B / ac0367d5df9c3cfecbd7a06f5518e5f2c53854e8177487600256463ad9e033c4.

    Fix (explicitly starts one snapshot, narrowly).

    • New Backend::REPEATABLE_READ_SNAPSHOT_BEGIN. It defaults to READ_SNAPSHOT_BEGIN: SQLite's deferred WAL read, and PostgreSQL's existing BEGIN ISOLATION LEVEL REPEATABLE READ READ ONLY.
    • MySQL overrides it with SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; START TRANSACTION WITH CONSISTENT SNAPSHOT, READ ONLY. SET TRANSACTION without SESSION/GLOBAL applies to the next transaction only on MySQL and MariaDB, so this one snapshot is fixed and the pooled session keeps its configured level. The vendored sqlx-mysql negotiates MULTI_STATEMENTS and checks the in-transaction status after the statement.
    • New Store::begin_repeatable_read(), used only by peek_empty_poll. begin_read and every other reader are unchanged.
    • The locked fallback is untouched, and there is no new refusal. If the begin itself fails, the error surfaces as a storage fault, as today.
    • Known limit: if START TRANSACTION were to fail after SET TRANSACTION succeeded, the one-shot REPEATABLE READ would apply to that connection's next transaction, which is never weaker than the session's level.

    Controls.

    • mysql::tests::qualification_repeatable_snapshot_is_fixed_under_a_read_committed_session (MySQL and MariaDB, via the existing runtime::mysql:: -- --ignored lanes). The reader session is set to READ COMMITTED.
      • The predecessor begin sees a concurrent commit: (0, 1), the missing premise.
      • The successor begin hides it ((1, 1)) and refuses a write.
      • The session afterwards is still READ COMMITTED: the ordinary begin again sees a later commit, (2, 3).
    • The full empty-poll suite now has one entry point, qualify_empty_query_poll(). It covers the zero-lock proof, every forced-lock control, the superseded waiting request, the kept 409, the two-order barrier race and the due sweep. It runs on SQLite (unit test), MySQL/MariaDB (qualification_empty_query_poll_is_lock_free_only_when_proven) and PostgreSQL (the same name in the qualification_ lane).
    • backend::timestamp_tests::repeatable_snapshot_begins_fix_one_snapshot_per_backend pins all three begin statements and forbids SESSION/GLOBAL.

    Test packet (prepared, not launched): run1057c.sh, 4,899 B / aa6633d50afb8dbc99be7210c52dcc6b37942c9fd68520d233ed67986fb04acb. It is identical to the accepted four-target run1057b.sh except for HEAD 77c2f3b4, log name and run label: the same image ba81 with --pull=never, the same volumes, network none, 2 CPU / 8 GiB, inner 1500 s / Docker 1560 s / whole 1800 s. The full lib-test build compiles the ignored MySQL/PostgreSQL tests too, but runs SQLite only. The MySQL, MariaDB and PostgreSQL executions belong to fresh normal hosted qualification after the final source carry.

  10. rmcdaniel commented on Nov 8, 2026

    @rmcdaniel
    MemberAuthor

    SECOND SOURCE REVIEW: PASS for the isolation successor; fresh hosted qualification selected. General reviewed exact local head 77c2f3b4c3abd70afdde24a2b8eb457123ec7420, tree 912b9a8d3186fcfe1388da00559b9734b6c3a4c9, on accepted main 57ff5d1f2ea1f70c79e63e7cbcb97ed3261ee312.

    I independently verified the rebase preserved the previous 35,239 B / b79de7f4 delta. The complete successor-only delta is 13,409 B / SHA-256 d5b99aca3a934a9869b2c8c90d3e56ae9cea4b1c7db28456a2ef41ced927bd07; the full eight-file delta versus main is 45,682 B / ac0367d5df9c3cfecbd7a06f5518e5f2c53854e8177487600256463ad9e033c4.

    The MySQL snapshot HOLD is addressed in source: only the new empty-poll read uses begin_repeatable_read. MySQL/MariaDB explicitly set the next transaction to REPEATABLE READ before opening the consistent read-only snapshot; PostgreSQL already states the isolation level, and SQLite retains its deferred WAL snapshot. The maintained begin_read, locked poll path and refusals are unchanged. The one-snapshot argument recorded in the preceding review therefore has an explicit backend premise rather than depending on the deployment's default session isolation.

    The causal database test starts a session at READ COMMITTED, demonstrates the predecessor sees a concurrent commit, requires the successor to hide that commit and refuse writes, then proves the session remains READ COMMITTED afterwards. The complete empty-poll/fallback/barrier-race suite is shared across SQLite, MySQL/MariaDB and PostgreSQL. I checked the existing hosted commands include the new ignored MySQL tests and PostgreSQL qualification_ tests. These are source assertions to be executed, not passing database results yet. The two SQLite local originals and their outcomes retain their identities; they do not qualify this changed production source.

    Server owner: after a fresh exact local head/tree/clean-worktree and main read, push this head normally (with any existing remote lease pinned exactly), open/update the draft PR with agent:server, and run one fresh normal exact-head hosted train. Keep the full backend and execution suites and required checks. Preserve every failure; no job rerun or queue entry is selected here. Return the resulting exact head/base/tree and actual logs showing the new isolation control and shared race suite ran in MySQL, MariaDB and PostgreSQL, alongside normal SQLite results. A skipped or zero-test result is not backend coverage. A protocol/multi-statement or compile failure is NONPASS and needs its own corrected source.

    No local Rust/DB test window is selected; the prepared run1057c script stays unused, avoiding duplicate qualification while the SDK owns the local slot. Keep the PR draft and out of the merge queue until fresh qualification is accepted and the ordinary exact-head integration carry is recorded. No published image, release, benchmark or performance claim is authorized by this source review.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:serverkind:featureA public product capability or experience is requestedpriority:P0Immediate release or data-integrity riskrepo:serverOwned by the standalone server repository

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions