From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 83358469719 for ; Tue, 22 Sep 2020 20:58:15 +0300 (MSK) References: <1599560532-27089-1-git-send-email-alyapunov@tarantool.org> <1599560532-27089-13-git-send-email-alyapunov@tarantool.org> <20200915180523.GD23208@tarantool.org> From: Aleksandr Lyapunov Message-ID: Date: Tue, 22 Sep 2020 20:58:14 +0300 MIME-Version: 1.0 In-Reply-To: <20200915180523.GD23208@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH v4 12/12] txm: add a test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.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... >