Re: [Tarantool-patches] [PATCH v4 12/12] txm: add a test

[Aleksandr Lyapunov <alyapunov@tarantool.org>]

Hi, thanks for the review.
I added the tests (triggers and recovery I'll do a bit later).
Found some bugs, here the fixes I made to make the tests pass.

diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c
index d72fa92..5aacd22 100644
--- a/src/box/memtx_tx.c
+++ b/src/box/memtx_tx.c
@@ -422,7 +422,7 @@ memtx_tx_story_gc_step()
               * actually delete the tuple, it must be deleted from
               * index.
               */
-            if (story->del_psn > 0) {
+            if (story->del_psn > 0 && story->space != NULL) {
                  struct index *index = story->space->index[i];
                  struct tuple *unused;
                  if (index_replace(index, story->tuple, NULL,
@@ -801,7 +801,6 @@ memtx_tx_history_rollback_stmt(struct txn_stmt *stmt)
              memtx_tx_story_unlink(story, i);
          }
          stmt->add_story->add_stmt = NULL;
-        memtx_tx_story_delete(stmt->add_story);
          stmt->add_story = NULL;
          tuple_unref(stmt->new_tuple);
      }
@@ -945,8 +944,22 @@ memtx_tx_history_prepare_stmt(struct txn_stmt *stmt)
      if (stmt->add_story != NULL)
          stmt->add_story->add_psn = stmt->txn->psn;

-    if (stmt->del_story != NULL)
+    if (stmt->del_story != NULL) {
          stmt->del_story->del_psn = stmt->txn->psn;
+        // Let's conflict all other deleting stories.
+        struct txn_stmt *dels = stmt->del_story->del_stmt;
+        while (dels != NULL) {
+            struct txn_stmt *next = dels->next_in_del_list;
+            if (dels != stmt) {
+                dels->del_story = NULL;
+                dels->next_in_del_list = NULL;
+            }
+            dels = next;
+        }
+        // Set the only deleting statement for that story.
+        stmt->del_story->del_stmt = stmt;
+        stmt->next_in_del_list = NULL;
+    }
  }

  ssize_t
@@ -1001,6 +1014,20 @@ memtx_tx_tuple_clarify_slow(struct txn *txn, 
struct space *space,
      return result;
  }

+void
+memtx_tx_on_space_delete(struct space *space)
+{
+    /* Just clear pointer to space, it will be handled in GC. */
+    while (!rlist_empty(&space->memtx_stories)) {
+        struct memtx_story *story
+            = rlist_first_entry(&space->memtx_stories,
+                        struct memtx_story,
+                        in_space_stories);
+        story->space = NULL;
+        rlist_del(&story->in_space_stories);
+    }
+}
+
  static void
  memtx_tx_story_delete(struct memtx_story *story)
  {
diff --git a/src/box/memtx_tx.h b/src/box/memtx_tx.h
index 6197d1b..aa204ac 100644
--- a/src/box/memtx_tx.h
+++ b/src/box/memtx_tx.h
@@ -310,6 +310,15 @@ memtx_tx_tuple_clarify(struct txn *txn, struct 
space *space,
                         is_prepared_ok);
  }

+/**
+ * Notify manager the a space is deleted.
+ * It's necessary because there is a chance that garbage collector hasn't
+ * deleted all stories of that space and in that case some actions of
+ * story's destructor are not applicable.
+ */
+void
+memtx_tx_on_space_delete(struct space *space);
+
  #if defined(__cplusplus)
  } /* extern "C" */
  #endif /* defined(__cplusplus) */
diff --git a/src/box/space.c b/src/box/space.c
index 1243932..6d1d771 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -38,6 +38,7 @@
  #include "user.h"
  #include "session.h"
  #include "txn.h"
+#include "memtx_tx.h"
  #include "tuple.h"
  #include "xrow_update.h"
  #include "request.h"
@@ -253,7 +254,7 @@ space_new_ephemeral(struct space_def *def, struct 
rlist *key_list)
  void
  space_delete(struct space *space)
  {
-    rlist_del(&space->memtx_stories);
+    memtx_tx_on_space_delete(space);
      assert(space->ck_constraint_trigger == NULL);
      for (uint32_t j = 0; j <= space->index_id_max; j++) {
          struct index *index = space->index_map[j];


On 15.09.2020 21:05, Nikita Pettik wrote:
> Uncovered test scenarious: more than two indexes; more than one space;
> unique secondary indexes; recovery; triggers; rollbacks...
>