Skip to content

Flaky: session_ten_logsql_limits cancellation counter race (1ms deadline) #49

Description

@awksedgreep

session_ten_logsql_limits_cancel_errors_and_direct_sql_reuse_the_reader (timeless-logs-api/tests/api_e2e.rs) fails intermittently on an unmodified tree.

Measured on a clean checkout of 91ece72, M1 Max, while other builds were running: 2 of 3 runs failed. Re-measured with an unrelated change applied: 3 of 4 failed. Same rate either way — this is not a regression from any recent commit, and I confirmed it by stashing before concluding.

The failure

thread 'session_ten_logsql_limits_cancel_errors_and_direct_sql_reuse_the_reader'
panicked at crates/timeless-logs-api/tests/api_e2e.rs:16379:5:
assertion failed: stats.api_query_cancelled > cancelled_before_replace_regexp

The 504 GATEWAY_TIMEOUT assertion just above it passes — the request does time out. What is racy is the bookkeeping: the test then polls storage.stats() up to 100 times at 5 ms intervals waiting for api_query_cancelled to increment, and gives up.

Why it is racy

The router is built with deadline: Duration::from_millis(1). A 1 ms deadline on a machine under load means the request can be cancelled at almost any point relative to the counter increment, and the 500 ms polling budget is not always enough for the cancellation to be observed and the in-flight count to return to zero. The same shape appears a few lines earlier for plain replace (:16344), which has not been observed failing but shares the pattern.

Why it matters

api_e2e is one of the --ignored real-extension suites the production gate runs (production-gate.yml, "Exact real-extension storage contracts"). A test that fails roughly two thirds of the time under load will fail the gate more often than it passes it, which trains people to re-run rather than read it — and #45 already flags these suites as easy to skip.

Suggested fix

Either give the deadline enough headroom to be deterministic (a few tens of ms rather than 1 ms), or make the assertion wait on the counter with a real timeout rather than a fixed 100 × 5 ms budget. The first is simpler; the second keeps the tight deadline if the intent is to exercise near-immediate cancellation.

Worth checking the sibling replace assertion at :16344 at the same time, since it is the same construction.

Activity

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions