From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp20.mail.ru (smtp20.mail.ru [94.100.179.251]) (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 93604469719 for ; Wed, 23 Sep 2020 14:07:21 +0300 (MSK) Date: Wed, 23 Sep 2020 11:07:19 +0000 From: Nikita Pettik Message-ID: <20200923110719.GC21612@tarantool.org> References: <1599560532-27089-1-git-send-email-alyapunov@tarantool.org> <1599560532-27089-13-git-send-email-alyapunov@tarantool.org> <20200915180523.GD23208@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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: Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org On 22 Sep 20:58, Aleksandr Lyapunov wrote: > 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. A few non-functional fixes: diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c index 55748ad64..b10440ae3 100644 --- a/src/box/memtx_tx.c +++ b/src/box/memtx_tx.c @@ -418,8 +418,8 @@ memtx_tx_story_gc_step() if (link->newer_story == NULL) { /* * We are at the top of the chain. That means - * that story->tuple is in index. If the story is - * actually delete the tuple, it must be deleted from + * that story->tuple is in index. If the story + * actually deletes the tuple, it must be deleted from * index. */ if (story->del_psn > 0 && story->space != NULL) { @@ -946,7 +946,7 @@ memtx_tx_history_prepare_stmt(struct txn_stmt *stmt) if (stmt->del_story != NULL) { stmt->del_story->del_psn = stmt->txn->psn; - // Let's conflict all other deleting stories. + /* 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; @@ -956,7 +956,7 @@ memtx_tx_history_prepare_stmt(struct txn_stmt *stmt) } dels = next; } - // Set the only deleting statement for that story. + /* Set the only deleting statement for that story. */ stmt->del_story->del_stmt = stmt; stmt->next_in_del_list = NULL; } @@ -1008,8 +1008,8 @@ memtx_tx_tuple_clarify_slow(struct txn *txn, struct space *space, if (!own_change) memtx_tx_track_read(txn, space, tuple); if (mk_index != 0) { - assert(false); /* TODO: multiindex */ - panic("multikey indexes are not supported int TX manager"); + unreachable(); /* TODO: multiindex */ + panic("multikey indexes are not supported in TX manager"); } return result; } diff --git a/src/box/memtx_tx.h b/src/box/memtx_tx.h index 25a203880..6aadad1f0 100644 --- a/src/box/memtx_tx.h +++ b/src/box/memtx_tx.h @@ -311,7 +311,7 @@ memtx_tx_tuple_clarify(struct txn *txn, struct space *space, } /** - * Notify manager the a space is deleted. + * Notify manager that the 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. Othewise LGTM > 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; > +    } >  }