Skip to content

Synchronize SAVEPOINT replay cancellation before mock RPC release - #975

Merged
apstndb merged 2 commits into
mainfrom
cursor/savepoint-cancel-test-sync-973-a066
Sep 14, 2026
Merged

apstndb merged 2 commits into
mainfrom
cursor/savepoint-cancel-test-sync-973-a066

Conversation

@apstndb

@apstndb apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Fixes #973.

TestSavepointReplayCancellationCleansUpWithoutCommit cancelled the command and immediately closed releaseSQL. That opened the mock's successful-response path before the cancelled context necessarily reached prepareSQL, so reconstruction could finish with err == nil (reconstruction error = <nil> on 0f7522b).

The heartbeat RPC harness already selects on ctx.Done() while the candidate SQL is blocked. This change keeps that barrier closed after cancel() so the in-flight RPC can only leave via cancellation. t.Cleanup still closes releaseSQL if the test fails or times out.

Assertions are unchanged: assigned candidate identity, cancelled cause, ended logical owner, cleared recovery, candidate rollback, and no Commit. No production code change, no sleeps or assertion retries, and draft #947 is untouched.

Based on origin/main f10a4da after an additive merge of #972.

Open in Web Open in Cursor 

cursoragent and others added 2 commits September 14, 2026 12:29
Keep the reconstruction SQL barrier closed after cancel() so
prepareSQL can only leave via ctx.Done(). Closing releaseSQL
immediately raced with cancellation and let reconstruction
succeed. Cleanup still unblocks the mock on test failure.

Fixes #973

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
…el-test-sync-973-a066

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

main (ec4e0be) #975 (8ef92e4) +/-
Coverage 86.7% 86.7% +0.0%
Code to Test Ratio 1:2.2 1:2.2 +0.0
Test Execution Time 1m51s 1m54s +3s
Details
  |                     | main (ec4e0be) | #975 (8ef92e4) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          86.7% |          86.7% | +0.0% |
  |   Files             |            112 |            112 |     0 |
  |   Lines             |          10405 |          10428 |   +23 |
+ |   Covered           |           9029 |           9051 |   +22 |
+ | Code to Test Ratio  |          1:2.2 |          1:2.2 |  +0.0 |
  |   Code              |          23263 |          23309 |   +46 |
+ |   Test              |          52022 |          52940 |  +918 |
- | Test Execution Time |          1m51s |          1m54s |   +3s |

Code coverage of files in pull request scope (88.7% → 88.8%)

Files Coverage +/- Status
internal/mycli/directed_read.go 97.0% +2.9% affected
internal/mycli/execute_partitioned.go 90.6% -0.9% affected
internal/mycli/session.go 85.1% +0.0% affected
internal/mycli/transaction_manager.go 87.8% +0.0% affected
internal/mycli/var_defs.go 96.0% +0.0% affected

Reported by octocov

@apstndb
apstndb marked this pull request as ready for review September 14, 2026 12:36
@apstndb

apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Independent review of HEAD d3ecdbf04ef72c356c56a0c168e3e00687f005b2: no actionable findings. Keeping the successful-response barrier closed after cancellation removes the competing success path while preserving failure cleanup and every cancellation/ownership assertion.

Independently validated this exact HEAD: the cancellation test passed 50 repetitions, the race-enabled test passed 10 repetitions, and make check passed with Docker. All nine applicable CI checks passed; the conditional Dependabot automerge job is skipped. No unresolved review feedback remains.

@apstndb
apstndb merged commit 92f986d into main Sep 14, 2026
10 checks passed
@apstndb
apstndb deleted the cursor/savepoint-cancel-test-sync-973-a066 branch September 14, 2026 12:44
cursor Bot pushed a commit that referenced this pull request Sep 14, 2026
…-36c5

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
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.

Synchronize SAVEPOINT replay cancellation before releasing the mock RPC

2 participants