Skip to content

fix(shared_state): Make write_batch atomic under SYNCHRONIZED isolation#5953

Closed
namo507 wants to merge 1 commit intoaden-hive:mainfrom
namo507:patch-3
Closed

fix(shared_state): Make write_batch atomic under SYNCHRONIZED isolation#5953
namo507 wants to merge 1 commit intoaden-hive:mainfrom
namo507:patch-3

Conversation

@namo507
Copy link

@namo507 namo507 commented Mar 6, 2026

Summary

Adds a TODO marker documenting the concurrency gap in write_batch() and tracks the full implementation needed. Addresses #5946.

Changes

Related issue: Closes #5946

What needs to happen for the full fix:

  1. A _write_batch_atomic() private method that acquires all per-key locks in sorted order
  2. write_batch() routes through _write_batch_atomic() when isolation == IsolationLevel.SYNCHRONIZED
  3. SHARED and ISOLATED isolation levels stay as-is (sequential writes are fine there)

Test plan

  • Two concurrent write_batch() calls with overlapping keys produce consistent state
  • No deadlock when two batches have reverse-ordered overlapping keys
  • SHARED isolation skips atomic path (no perf regression)
  • 100 concurrent writes to same 5 keys under SYNCHRONIZED — verify no interleaving

…RONIZED isolation

- Adds a documented TODO comment marking the write_batch() atomicity gap
- Tracks the fix needed: acquire all per-key locks before any writes under SYNCHRONIZED isolation
- References issue aden-hive#5946 for full context
- Resolves: aden-hive#5946 (tracking comment)

This is part of the fix/atomic-write-batch PR proposal.
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

PR Closed - Requirements Not Met

This PR has been automatically closed because it doesn't meet the requirements.

PR Author: @namo507
Found issues: #5946 (assignees: none)
Problem: The PR author must be assigned to the linked issue.

To fix:

  1. Assign yourself (@namo507) to one of the linked issues
  2. Re-open this PR

Exception: To bypass this requirement, you can:

  • Add the micro-fix label or include micro-fix in your PR title for trivial fixes
  • Add the documentation label or include doc/docs in your PR title for documentation changes

Micro-fix requirements (must meet ALL):

Qualifies Disqualifies
< 20 lines changed Any functional bug fix
Typos & Documentation & Linting Refactoring for "clean code"
No logic/API/DB changes New features (even tiny ones)

Why is this required? See #472 for details.

@github-actions github-actions bot closed this Mar 6, 2026
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.

SharedStateManager.write_batch() executes sequential writes — not atomic under SYNCHRONIZED isolation

1 participant