Unwedge dolt_commit after a refused merge in --no-commit state - #2549
Merged
Conversation
A second dolt_merge in a session that hydrated persisted --no-commit merge state correctly refuses with uncommitted changes, but EnsureWriteTxnAndSavepoints had already started a btree write from the read-only SELECT VM. Halt skips RollbackAll in that case, so the connection stays TXN_WRITE with autoCommit still 1. The next dolt_commit then issues COMMIT and gets "cannot commit - no transaction is active". Rollback the autocommit write on every post-EnsureWriteTxn refusal. The following dolt_commit then records the 2-parent merge on the first try, matching Dolt. Fixes #2539 Co-Authored-By: Grok 4.6 <noreply@x.ai>
|
SummaryCoverage spans normal and failed merge flows, branch-history and working-data preservation, transaction and connection recovery, explicit rollback behavior, and remote update failures. It includes both happy-path business behavior and adversarial edge cases around retries, missing sources, refusals, and pending changes. Safe to merge — all exercised behaviors passed, and the apparent pending-merge loss findings were independently determined not to reflect an actual defect or a promise of the code. No PR-attributable regressions or unresolved failures remain. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
RollbackAll on an early dolt_merge refusal restored the still-open write snapshot and persisted it. That wiped earlier autocommit merges in the same connection: merge A, merge bogus, merge B left only the pre-A working set. Error-recovery oracles then reported 1 row where Dolt had 3 or 4. These refusals have not mutated. sqlite3BtreeCommit ends the write so halt is not stuck on TXN_WRITE (the #2539 wedge) without rewinding HEAD. The fan-in bogus-name sequence keeps every successful merge. Co-Authored-By: Grok 4.6 <noreply@x.ai>
Contributor
DoltLite performance vs PR base
blobpk details
compositepk details
int details
textpk details
vc details
All relative performance gates passed. |
|
Diff SummaryCoverage focused on database merge and transaction behavior, including successful merges, missing or refused sources, dirty working data, rollback and savepoints, repeated recovery attempts, persistence after reopening, and concurrent access. These scenarios exercise normal workflows as well as adversarial and edge-case handling around data safety, transaction ownership, and recovery. Safe to merge — the exercised behaviors are healthy, with no PR-attributable regressions, new failures, or previously flagged failures still failing. The untested previously passing cases are coverage gaps rather than merge blockers, so they are flag-for-later observations. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
Contributor
DoltLite source coverage
Merged 202 pooled raw profiles from the distributed Linux correctness jobs. Per-file coverage (98 files)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2539.
A session that reopens a
--no-commitmerge correctly refuses a seconddolt_merge(uncommitted changes — commit or reset before merging). That refusal left a btree write transaction open:doltliteEnsureWriteTxnAndSavepointsstarts a write from a read-only SELECT VM, and halt then skipsRollbackAll. The nextdolt_commitin the same session sawsqlite3_txn_state != NONEwithautoCommitstill 1, issuedCOMMIT, and failed withcannot commit - no transaction is active. A retry worked.On autocommit, every
doltliteMergeReferror afterEnsureWriteTxnAndSavepointsnowsqlite3RollbackAll+sqlite3CloseSavepoints. UserBEGINsessions are left alone.test/doltlite_merge.shcovers the issue repro: refuse, then a first-try commit that is a 2-parent merge.Co-Authored-By: Grok 4.6 noreply@x.ai