diff --git a/src/mvcc/version_list.hpp b/src/mvcc/version_list.hpp index 7ce2a33be..2c5de38bc 100644 --- a/src/mvcc/version_list.hpp +++ b/src/mvcc/version_list.hpp @@ -85,7 +85,6 @@ class VersionList { * visible anymore. If none exists to_delete will point to nullptr. */ std::pair GcDeleted(const Id &id, tx::Engine &engine) { - // nullptr // | // [v1] ... all of this gets deleted! @@ -96,14 +95,14 @@ class VersionList { // | | Jump backwards until you find the oldest visible // [VerList] ----+ record, or you reach the end of the list // - + auto current = head.load(); T *head_of_deletable_records = current; T *oldest_visible_record = nullptr; while (current) { if (!current->is_not_visible_from(id, engine)) - oldest_visible_record = current; - current = current->next(); + oldest_visible_record = current; + current = current->next(); } if (oldest_visible_record) head_of_deletable_records = oldest_visible_record->next(); @@ -131,7 +130,8 @@ class VersionList { // further than this record and // that's why it's safe to set // next to nullptr. - // Calling destructor of head_of_deletable_records will clean everything older + // Calling destructor of head_of_deletable_records will clean everything + // older // than this record since they are called recursively. return std::make_pair(false, head_of_deletable_records); } @@ -192,14 +192,29 @@ class VersionList { } } + /** + * Looks for the first visible record seen by this transaction. If the current + * transaction has already created new record in the current command then that + * record is returned, else first older visible record is updated. New record + * becomes head of the version list and it is returned. There should always be + * older visible record when this update is called. + * + * @param t The transaction + */ T *update(tx::Transaction &t) { debug_assert(head != nullptr, "Head is nullptr on update."); - auto record = find(t); + T *old_record = nullptr; + T *new_record = nullptr; + find_set_old_new(t, old_record, new_record); + + // check if current transaction in current cmd has + // already updated version list + if (new_record) return new_record; // check if we found any visible records - permanent_assert(record != nullptr, "Updating nullptr record"); + permanent_assert(old_record != nullptr, "Updating nullptr record"); - return update(record, t); + return update(old_record, t); } void remove(tx::Transaction &t) { diff --git a/tests/unit/mvcc_find_update_common.hpp b/tests/unit/mvcc_find_update_common.hpp index c8f770654..0413db1ba 100644 --- a/tests/unit/mvcc_find_update_common.hpp +++ b/tests/unit/mvcc_find_update_common.hpp @@ -76,7 +76,7 @@ class Mvcc : public ::testing::Test { // - TX_BEGIN sets the transaction variable tX #define T2_FIND __attribute__((unused)) auto v2 = version_list.find(*t2) #define T3_FIND __attribute__((unused)) auto v3 = version_list.find(*t3) -#define T4_FIND version_list.find(*t4) +#define T4_FIND __attribute__((unused)) auto v4 = version_list.find(*t4) #define T2_UPDATE __attribute__((unused)) auto v2 = version_list.update(*t2) #define T3_UPDATE __attribute__((unused)) auto v3 = version_list.update(*t3) #define T2_COMMIT t2->commit(); diff --git a/tests/unit/mvcc_one_transaction.cpp b/tests/unit/mvcc_one_transaction.cpp new file mode 100644 index 000000000..661e8e5c9 --- /dev/null +++ b/tests/unit/mvcc_one_transaction.cpp @@ -0,0 +1,110 @@ +#include "mvcc_find_update_common.hpp" + +#undef T2_FIND +#define T2_FIND version_list.find(*t2) + +#undef EXPECT_CRE +#undef EXPECT_EXP +#define EXPECT_CRE(record, transaction, command) \ + EXPECT_EQ(record->tx.cre(), id##transaction); \ + EXPECT_EQ(record->cmd.cre(), command) +#define EXPECT_EXP(record, transaction, command) \ + EXPECT_EQ(record->tx.exp(), id##transaction); \ + EXPECT_EQ(record->cmd.exp(), command) + +// IMPORTANT: look definiton of EXPECT_CRE and EXPECT_EXP macros in +// tests/mvcc_find_update_common.hpp. Numbers in those macros represent +// transaction ids when transactions where created. + +TEST_F(Mvcc, UpdateNotAdvanceUpdate) { + T2_UPDATE; + EXPECT_EQ(T2_FIND, v1); + auto v2_2 = version_list.update(*t2); + EXPECT_NXT(v2, v1); + EXPECT_EQ(v2, v2_2); + EXPECT_CRE(v2, 2, 1); + EXPECT_EXP(v2, 0, 0); + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(2); +} + +TEST_F(Mvcc, UpdateNotAdvanceRemove) { + T2_UPDATE; + EXPECT_EQ(T2_FIND, v1); + T2_REMOVE; + EXPECT_NXT(v2, v1); + EXPECT_CRE(v2, 2, 1); + EXPECT_EXP(v2, 0, 0); + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(2); +} + +TEST_F(Mvcc, RemoveNotAdvanceUpdate) { + T2_REMOVE; + EXPECT_EQ(T2_FIND, v1); + T2_UPDATE; + EXPECT_NXT(v2, v1); + EXPECT_CRE(v2, 2, 1); + EXPECT_EXP(v2, 0, 0); + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(2); +} + +TEST_F(Mvcc, RemoveNotAdvanceRemove) { + T2_REMOVE; + EXPECT_EQ(T2_FIND, v1); + T2_REMOVE; + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(1); +} + +TEST_F(Mvcc, UpdateAdvanceUpdate) { + T2_UPDATE; + EXPECT_EQ(T2_FIND, v1); + engine.advance(t2->id); + EXPECT_EQ(T2_FIND, v2); + auto v2_2 = version_list.update(*t2); + EXPECT_NXT(v2, v1); + EXPECT_NXT(v2_2, v2); + EXPECT_CRE(v2, 2, 1); + EXPECT_EXP(v2, 2, 2); + EXPECT_CRE(v2_2, 2, 2); + EXPECT_EXP(v2_2, 0, 0); + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(3); +} + +TEST_F(Mvcc, UpdateAdvanceRemove) { + T2_UPDATE; + EXPECT_EQ(T2_FIND, v1); + engine.advance(t2->id); + EXPECT_EQ(T2_FIND, v2); + T2_REMOVE; + EXPECT_NXT(v2, v1); + EXPECT_CRE(v2, 2, 1); + EXPECT_EXP(v2, 2, 2); + EXPECT_CRE(v1, 1, 1); + EXPECT_EXP(v1, 2, 1); + EXPECT_SIZE(2); +} + +TEST_F(Mvcc, RemoveAdvanceUpdate) { + T2_REMOVE; + EXPECT_EQ(T2_FIND, v1); + engine.advance(t2->id); + EXPECT_EQ(T2_FIND, nullptr); + EXPECT_DEATH(T2_UPDATE, ".*nullptr.*"); +} + +TEST_F(Mvcc, RemoveAdvanceRemove) { + T2_REMOVE; + EXPECT_EQ(T2_FIND, v1); + engine.advance(t2->id); + EXPECT_EQ(T2_FIND, nullptr); + EXPECT_DEATH(T2_REMOVE, ".*nullptr.*"); +} diff --git a/tests/unit/mvcc_parallel_update.cpp b/tests/unit/mvcc_parallel_update.cpp index 43dc7eaab..71dcd1923 100644 --- a/tests/unit/mvcc_parallel_update.cpp +++ b/tests/unit/mvcc_parallel_update.cpp @@ -1,5 +1,12 @@ #include "mvcc_find_update_common.hpp" +#undef T4_FIND +#define T4_FIND version_list.find(*t4) + + +// IMPORTANT: look definiton of EXPECT_CRE and EXPECT_EXP macros in +// tests/mvcc_find_update_common.hpp. Numbers in those macros represent +// transaction ids when transactions where created. // **************************************************************** // * CASE 1: T3 starts after T2 ends.