From aff901baeaaf8d55115116108ac1eef757cd6955 Mon Sep 17 00:00:00 2001 From: avi Date: Fri, 14 Apr 2023 21:59:35 +0530 Subject: [PATCH 1/3] bugfix: make committed rows visibile (fixes #15) --- core/mvcc/database/src/database.rs | 59 +++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/core/mvcc/database/src/database.rs b/core/mvcc/database/src/database.rs index 6a1c7ac1f..05a662435 100644 --- a/core/mvcc/database/src/database.rs +++ b/core/mvcc/database/src/database.rs @@ -435,7 +435,7 @@ fn is_end_visible(txs: &HashMap, tx: &Transaction, rv: &RowVe Some(TxTimestampOrID::TxID(rv_end)) => { let te = txs.get(&rv_end).unwrap(); match te.state { - TransactionState::Active => tx.tx_id == te.tx_id && rv.end.is_none(), + TransactionState::Active => tx.tx_id != te.tx_id, TransactionState::Preparing => todo!(), TransactionState::Committed => todo!(), TransactionState::Aborted => todo!(), @@ -723,4 +723,61 @@ mod tests { let row = db.read(tx4, 1).await.unwrap().unwrap(); assert_eq!(tx2_row, row); } + + // Test for the visibility to check if a new transaction can see old committed values. + // This test checks for the typo present in the paper, explained in https://github.com/penberg/mvcc-rs/issues/15 + #[traced_test] + #[tokio::test] + async fn test_committed_visibility() { + let clock = LocalClock::default(); + let db = Database::>::new(clock); + + // let's add $10 to my account since I like money + let tx1 = db.begin_tx().await; + let tx1_row = Row { + id: 1, + data: "10".to_string(), + }; + db.insert(tx1, tx1_row.clone()).await.unwrap(); + db.commit_tx(tx1).await.unwrap(); + + // but I like more money, so let me try adding $10 more + let tx2 = db.begin_tx().await; + let tx2_row = Row { + id: 1, + data: "20".to_string(), + }; + assert!(db.update(tx2, tx2_row.clone()).await.unwrap()); + + // can I check how much money I have? + let tx3 = db.begin_tx().await; + let row = db.read(tx3, 1).await.unwrap().unwrap(); + assert_eq!(tx1_row, row); + } + + // Test to check if a older transaction can see (un)committed future rows + #[traced_test] + #[tokio::test] + async fn test_future_row() { + let clock = LocalClock::default(); + let db = Database::>::new(clock); + + let tx1 = db.begin_tx().await; + + let tx2 = db.begin_tx().await; + let tx2_row = Row { + id: 1, + data: "10".to_string(), + }; + db.insert(tx2, tx2_row.clone()).await.unwrap(); + + // transaction in progress, so tx1 shouldn't be able to see the value yet + let row = db.read(tx1, 1).await.unwrap(); + assert_eq!(row, None); + + // lets commit the transaction and check if tx1 can see it now + db.commit_tx(tx2).await.unwrap(); + let row = db.read(tx1, 1).await.unwrap(); + assert_eq!(row, None); + } } From e00617748069a363b0d6656d30daa50c48853fcf Mon Sep 17 00:00:00 2001 From: avi Date: Fri, 14 Apr 2023 22:06:55 +0530 Subject: [PATCH 2/3] fix typos --- core/mvcc/database/src/database.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/mvcc/database/src/database.rs b/core/mvcc/database/src/database.rs index 05a662435..a2b6c235a 100644 --- a/core/mvcc/database/src/database.rs +++ b/core/mvcc/database/src/database.rs @@ -771,11 +771,11 @@ mod tests { }; db.insert(tx2, tx2_row.clone()).await.unwrap(); - // transaction in progress, so tx1 shouldn't be able to see the value yet + // transaction in progress, so tx1 shouldn't be able to see the value let row = db.read(tx1, 1).await.unwrap(); assert_eq!(row, None); - // lets commit the transaction and check if tx1 can see it now + // lets commit the transaction and check if tx1 can see it db.commit_tx(tx2).await.unwrap(); let row = db.read(tx1, 1).await.unwrap(); assert_eq!(row, None); From 87b9b272159a3d8816d9e26a1a071a1fb6adb01e Mon Sep 17 00:00:00 2001 From: avi Date: Fri, 14 Apr 2023 22:56:07 +0530 Subject: [PATCH 3/3] check if tx can see its own updates --- core/mvcc/database/src/database.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/mvcc/database/src/database.rs b/core/mvcc/database/src/database.rs index a2b6c235a..ecfe8e743 100644 --- a/core/mvcc/database/src/database.rs +++ b/core/mvcc/database/src/database.rs @@ -748,6 +748,8 @@ mod tests { data: "20".to_string(), }; assert!(db.update(tx2, tx2_row.clone()).await.unwrap()); + let row = db.read(tx2, 1).await.unwrap().unwrap(); + assert_eq!(row, tx2_row); // can I check how much money I have? let tx3 = db.begin_tx().await;