Added mvcc tests with one transaction and modified version list update function

Summary: Modified version list update, added mvcc tests for one transaction

Reviewers: dgleich, florijan

Reviewed By: dgleich, florijan

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D360

src/mvcc/version_list.hpp

@@ -85,7 +85,6 @@ class VersionList {
    * visible anymore. If none exists to_delete will point to nullptr.
   */
   std::pair<bool, T *> 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) {

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();

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.*");
+}

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.