From 30596de7414ce8d21fb639fe4ea00f7f415652d0 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 08:28:45 +0300 Subject: [PATCH 1/6] mvcc: dont set tx state to commit before actually committing --- core/mvcc/database/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index 596ff3b9e..1c3bac202 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -803,11 +803,6 @@ impl StateTransition for CommitStateMachine { } CommitState::Commit { end_ts } => { let mut log_record = LogRecord::new(*end_ts); - let tx = mvcc_store.txs.get(&self.tx_id).unwrap(); - let tx_unlocked = tx.value(); - tx_unlocked - .state - .store(TransactionState::Committed(*end_ts)); for id in &self.write_set { if let Some(row_versions) = mvcc_store.rows.get(id) { let mut row_versions = row_versions.value().write(); @@ -909,6 +904,11 @@ impl StateTransition for CommitStateMachine { return Ok(TransitionResult::Continue); } CommitState::CommitEnd { end_ts } => { + let tx = mvcc_store.txs.get(&self.tx_id).unwrap(); + let tx_unlocked = tx.value(); + tx_unlocked + .state + .store(TransactionState::Committed(*end_ts)); // We have now updated all the versions with a reference to the // transaction ID to a timestamp and can, therefore, remove the // transaction. Please note that when we move to lockless, the From ed06c7c423d1681e20dd1d489fcf5c68a17f8a06 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 08:29:24 +0300 Subject: [PATCH 2/6] mvcc: fix hang when non-concurrent tx holds write lock --- core/mvcc/database/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index 1c3bac202..8be6b6495 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -803,6 +803,10 @@ impl StateTransition for CommitStateMachine { } CommitState::Commit { end_ts } => { let mut log_record = LogRecord::new(*end_ts); + if !mvcc_store.is_exclusive_tx(&self.tx_id) && mvcc_store.has_exclusive_tx() { + // A non-CONCURRENT transaction is holding the exclusive lock, we must abort. + return Err(LimboError::WriteWriteConflict); + } for id in &self.write_set { if let Some(row_versions) = mvcc_store.rows.get(id) { let mut row_versions = row_versions.value().write(); From c289ab90bc6b1f9204c18835287897716110ccfe Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 08:29:48 +0300 Subject: [PATCH 3/6] mvcc: fix trying to end pager tx in rollback --- core/mvcc/database/mod.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index 8be6b6495..a9ee35382 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -1648,7 +1648,7 @@ impl MvStore { pub fn rollback_tx( &self, tx_id: TxID, - pager: Arc, + _pager: Arc, connection: &Connection, ) -> Result<()> { let tx_unlocked = self.txs.get(&tx_id).unwrap(); @@ -1659,14 +1659,10 @@ impl MvStore { tracing::trace!("abort(tx_id={})", tx_id); let write_set: Vec = tx.write_set.iter().map(|v| *v.value()).collect(); - let pager_rollback_done = if self.is_exclusive_tx(&tx_id) { + if self.is_exclusive_tx(&tx_id) { self.commit_coordinator.pager_commit_lock.unlock(); self.release_exclusive_tx(&tx_id); - pager.io.block(|| pager.end_tx(true, connection))?; - true - } else { - false - }; + } for ref id in write_set { if let Some(row_versions) = self.rows.get(id) { @@ -1688,9 +1684,6 @@ impl MvStore { let tx = tx_unlocked.value(); tx.state.store(TransactionState::Terminated); tracing::trace!("terminate(tx_id={})", tx_id); - if !pager_rollback_done { - pager.end_read_tx()?; - } // FIXME: verify that we can already remove the transaction here! // Maybe it's fine for snapshot isolation, but too early for serializable? self.txs.remove(&tx_id); From 8555d81a62e114b156048d311a01d1fc5cb1255f Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 08:58:18 +0300 Subject: [PATCH 4/6] mvcc: keep existing begin timestamp when upgrading mv tx to exclusive --- core/mvcc/database/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index a9ee35382..54cd1d0cf 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -1455,7 +1455,11 @@ impl MvStore { ) -> Result> { let is_logical_log = self.storage.is_logical_log(); let tx_id = maybe_existing_tx_id.unwrap_or_else(|| self.get_tx_id()); - let begin_ts = self.get_timestamp(); + let begin_ts = if let Some(tx_id) = maybe_existing_tx_id { + self.txs.get(&tx_id).unwrap().value().begin_ts + } else { + self.get_timestamp() + }; self.acquire_exclusive_tx(&tx_id)?; From 7410200a9f51e3bf917a8ea5b672c23cfe81c714 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 09:17:55 +0300 Subject: [PATCH 5/6] mvcc: add higher weight for BEGIN CONCURRENT in fuzz test --- tests/integration/fuzz_transaction/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/fuzz_transaction/mod.rs b/tests/integration/fuzz_transaction/mod.rs index b8667da2a..f4c39d375 100644 --- a/tests/integration/fuzz_transaction/mod.rs +++ b/tests/integration/fuzz_transaction/mod.rs @@ -497,8 +497,8 @@ async fn test_multiple_connections_fuzz_mvcc() { mvcc_enabled: true, max_num_connections: 8, query_gen_options: QueryGenOptions { - weight_begin_deferred: 8, - weight_begin_concurrent: 8, + weight_begin_deferred: 4, + weight_begin_concurrent: 12, weight_commit: 8, weight_rollback: 8, weight_checkpoint: 0, From 22a8992b6b24a2dbe1982cc218974ed8134ffe4e Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 19 Sep 2025 09:18:13 +0300 Subject: [PATCH 6/6] mvcc: un-ignore mvcc fuzz test --- tests/integration/fuzz_transaction/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/fuzz_transaction/mod.rs b/tests/integration/fuzz_transaction/mod.rs index f4c39d375..cc8e35724 100644 --- a/tests/integration/fuzz_transaction/mod.rs +++ b/tests/integration/fuzz_transaction/mod.rs @@ -490,7 +490,6 @@ async fn test_multiple_connections_fuzz() { } #[tokio::test] -#[ignore = "MVCC is currently under development, it is expected to fail"] // Same as test_multiple_connections_fuzz, but with MVCC enabled. async fn test_multiple_connections_fuzz_mvcc() { let mvcc_fuzz_options = FuzzOptions {