Skip to content

Unify concurrency request chain identity - #333

Draft
udnay wants to merge 2 commits into
mainfrom
codex/unify-enqueue-paths-without-tombstone-conflicts
Draft

Unify concurrency request chain identity#333
udnay wants to merge 2 commits into
mainfrom
codex/unify-enqueue-paths-without-tombstone-conflicts

Conversation

@udnay

@udnay udnay commented May 28, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Eliminate holder leaks and limit-bypass bugs by giving queued concurrency requests a durable chain identity so scanner resumption can continue the same limit-walk instead of inventing a synthetic id.
  • Make the enqueue/scanner/ticket flows share a single resumeable walker model so immediate grants, queued requests, and future RequestTicket promotions behave consistently.
  • Keep the change focused: only unify request payloads/identity and scanner resumption behavior, leaving tombstone/task-key-start-ms conflict handling to the planned follow-up.

Description

  • Persist chain resume state in request intents: added task_id, held_queues, next_limit_index, and serialized limits to ConcurrencyAction::EnqueueTask and the FlatBuffer schema, plus codec helpers to encode/decode limit entries.
  • Stop writing interim runnable RunAttempt on immediate per-limit grants; append_grant_edits now writes only the holder record and the final RunAttempt is written by the unified walker once the chain fully resolves.
  • Reworked grant scanner (process_grants) to decode the extended EnqueueTask, validate payload, reserve slots across possibly multiple queues while resuming the chain, emit new request edits when the resumed walk pauses on a later queue, derive per-pass request-counter deltas from walker actions, and roll back all in-memory reservations from the pass on write failure.
  • Caller-side changes: enqueue_limit_task_at_index forwards held-queues/next-index to concurrency helpers so queued intents carry the full resume payload; query code prefers decoding the request payload's task_id when available; added a regression test scanner_resumes_paused_chain_with_original_task_id_and_all_holders and adjusted existing codec/tests to the new payload shape.
  • Edge cases intentionally out of scope: tombstone / task_key_start_ms conflicts (handled by separate task_key_start_ms work), RequestTicket future scheduling semantics are not fully expanded to carry full chain state, cancel/reimport cleanup for EnqueueTask-held queues requires separate rework, scanner resumption uses durable/default floating capacity rather than inline floating refresh, and queue/index mismatch validations / exhaustive malformed-payload policies are minimal in this change.

Testing

  • Ran quick static checks: git diff --check and cargo fmt --check — both passed.
  • Attempted cargo check --tests / cargo test but the build script invokes flatc (FlatBuffers compiler) and the environment lacks flatc, causing the build script to panic and tests to be blocked; see build error "failed to run flatc … No such file or directory".
  • Added/updated regression tests and codec round-trip coverage (notably scanner_resumes_paused_chain_with_original_task_id_and_all_holders, codec roundtrips for the extended EnqueueTask), but their execution was blocked by the missing flatc in CI/local environment.
  • Manual code-level review and local cargo fmt/linting performed; no formatting or obvious compile-time syntax issues remain beyond the build-tool dependency failure.

Codex Task

@udnay
udnay changed the base branch from yo/refactor-enqueu to main May 30, 2026 15:30
@udnay

udnay commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

This change is part of the following stack:

Change managed by git-spice.

@udnay
udnay force-pushed the codex/unify-enqueue-paths-without-tombstone-conflicts branch from 3ea445c to 5d18a17 Compare May 30, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant