From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (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 8C86441C5DB for ; Wed, 1 Jul 2020 02:00:39 +0300 (MSK) From: Vladislav Shpilevoy References: <38933b569988f89ad124e71e49b460698db5e4a0.1593472477.git.v.shpilevoy@tarantool.org> Message-ID: <88dff1c0-8ecb-e42e-147e-e97a916eba8c@tarantool.org> Date: Wed, 1 Jul 2020 01:00:37 +0200 MIME-Version: 1.0 In-Reply-To: <38933b569988f89ad124e71e49b460698db5e4a0.1593472477.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 04/19] replication: make sync transactions wait quorum List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, sergepetrenko@tarantool.org Merged this diff into the commit, in scope of https://github.com/tarantool/tarantool/issues/5123. Key difference is that txn_limbo_assign_lsn() has nothing to do with ACKs now. Indeed, it is called even on replica, it should not care about ACKs. The local WAL write on master now is marked as one of the ACKs, explicitly, by calling txn_limbo_ack(). Also ther was a bug, that in case the transaction is already complete at the moment of calling txn_limbo_wait_complete(), it didn't do proper cleanup. In this patch it does not matter, but in the later patches becomes a bug. This is solved by making one place where this funtion always ends and using goto to it. ==================== diff --git a/src/box/txn.c b/src/box/txn.c index 6cfa98212..eaba90274 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -702,8 +702,10 @@ txn_commit(struct txn *txn) return -1; } if (is_sync) { - txn_limbo_assign_lsn(&txn_limbo, limbo_entry, - req->rows[req->n_rows - 1]->lsn); + int64_t lsn = req->rows[req->n_rows - 1]->lsn; + txn_limbo_assign_lsn(&txn_limbo, limbo_entry, lsn); + /* Local WAL write is a first 'ACK'. */ + txn_limbo_ack(&txn_limbo, txn_limbo.instance_id, lsn); txn_limbo_wait_complete(&txn_limbo, limbo_entry); } if (!txn_has_flag(txn, TXN_IS_DONE)) { diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c index 9de91db93..c9bb2b7ff 100644 --- a/src/box/txn_limbo.c +++ b/src/box/txn_limbo.c @@ -96,9 +96,9 @@ txn_limbo_assign_lsn(struct txn_limbo *limbo, struct txn_limbo_entry *entry, int64_t lsn) { assert(limbo->instance_id != REPLICA_ID_NIL); + assert(entry->lsn == -1); + assert(lsn > 0); entry->lsn = lsn; - ++entry->ack_count; - vclock_follow(&limbo->vclock, limbo->instance_id, lsn); } static bool @@ -125,14 +125,13 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry) assert(entry->lsn > 0); assert(!txn_has_flag(txn, TXN_IS_DONE)); assert(txn_has_flag(txn, TXN_WAIT_ACK)); - if (txn_limbo_check_complete(limbo, entry)) { - txn_limbo_remove(limbo, entry); - return; - } + if (txn_limbo_check_complete(limbo, entry)) + goto complete; bool cancellable = fiber_set_cancellable(false); while (!txn_limbo_entry_is_complete(entry)) fiber_yield(); fiber_set_cancellable(cancellable); +complete: // TODO: implement rollback. // TODO: implement confirm. assert(!entry->is_rollback);