Skip to content

Fence uncertain Matching task writes before advancing readers - #12010

Draft
taylan-oai wants to merge 1 commit into
temporalio:mainfrom
taylan-oai:dev/taylan/reconcile-uncertain-matching-writes
Draft

taylan-oai wants to merge 1 commit into
temporalio:mainfrom
taylan-oai:dev/taylan/reconcile-uncertain-matching-writes

Conversation

@taylan-oai

Copy link
Copy Markdown

Summary

Fence uncertain non-fair Matching task writes before advancing the readable task-ID boundary, so a delayed write cannot land behind the reader and acknowledgment positions.

Problem

CreateTasks currently advances maxReadLevel even when persistence returns an error with an unknown commit outcome. An empty read can then advance the acknowledgment position past a task that commits later. A subsequent successful append can expose the same gap, so leaving the boundary unchanged for only the failed call is insufficient.

Approach

Keep the uncertain range closed and reject further writes until the writer renews its lease. The successful range-ID update fences the previous write before readers can cross its IDs. Both classic and priority writers perform this recovery immediately, allocate from the new block, and wake their readers. If recovery fails, the queue unloads with the uncertain range still closed.

Preserve the approximate backlog count while those IDs are hidden. Definite persistence rejections keep their existing accounting and ID-consumption behavior. Fair task writers are outside this change.

Validation

  • Native changed-code lint passed.
  • The new regression fails on the base code because an empty read acknowledges a still-pending task. It passes after the change, including both commit orders and conservative backlog accounting.
  • Both writer variants pass recovery, reader notification, fresh-ID allocation, and failed-fence tests; repeated 20 times with the race detector.
  • Native Cassandra tests pass commit-before-fence and fence-before-commit cases, with metadata updates on and off; repeated three times with the race detector.
  • All classic, priority, and fair backlog suites pass.
  • The full Matching package was run twice. Existing priority approximate-backlog assertions failed, including TestLesserNumberOfPollersThanTasksNoDBErrors. An overlay using the unchanged production files from the base commit reproduced these failures. The first full run also hit the existing fake-time TestPerKeyRateLimit timing bound.

Risks, rollout, and scope

An uncertain write now incurs a lease renewal using the existing 30-second retry policy and consumes the remainder of its task-ID block. The original append still fails, so caller retries can produce duplicate logical tasks as before.

The Cassandra test gates the old write before it enters the native store. It exercises the real range-ID compare-and-set and task reads on one Cassandra node; it does not inject replica failures or pause an already-accepted Paxos proposal. SQL fencing was checked in the existing transaction and task-queue lock implementation, without a new SQL integration run. No schema change or migration is required.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dnr

dnr commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

I don't think this is actually a bug: for a failed AddTask call, the semantics are that the task may or may not be dispatched. In any case, the caller should retry (if it wants the task to definitely be dispatched). Duplicate dispatch if the delayed write is actually read later is protected by RecordTaskStarted (and even if that layer wasn't there, tasks are idempotent). If it's skipped over, then the AddTask retry will handle it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants