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 8E40042EF5C for ; Thu, 11 Jun 2020 16:04:08 +0300 (MSK) References: <3210e1e6f867cfd1c1f65e05f28a32deae63c172.1591701695.git.sergepetrenko@tarantool.org> <7228506b-2eff-befb-43b7-f933a8844867@tarantool.org> <1a8d2474-b84e-c5c9-6c4b-5c07f1fb7055@tarantool.org> From: Vladislav Shpilevoy Message-ID: <282171ab-8c6e-3b9f-7c81-8c29becde72c@tarantool.org> Date: Thu, 11 Jun 2020 15:04:06 +0200 MIME-Version: 1.0 In-Reply-To: <1a8d2474-b84e-c5c9-6c4b-5c07f1fb7055@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH 8/8] replication: write and read CONFIRM entries List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko , sergos@tarantool.org, gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org >>> diff --git a/src/box/txn.c b/src/box/txn.c >>> index a65100b31..3b331fecc 100644 >>> --- a/src/box/txn.c >>> +++ b/src/box/txn.c> @@ -510,13 +518,6 @@ txn_journal_entry_new(struct txn *txn) >>>       struct xrow_header **remote_row = req->rows; >>>       struct xrow_header **local_row = req->rows + txn->n_applier_rows; >>>       bool is_sync = false; >>> -    /* >>> -     * Only local transactions, originated from the master, >>> -     * can enter 'waiting for acks' state. It means, only >>> -     * author of the transaction can collect acks. Replicas >>> -     * consider it a normal async transaction so far. >>> -     */ >>> -    bool is_local = true; >> I squashed is_local removal into the first commits. So like it didn't >> exist at all. >> >> Why did you remove it, btw? I applied the removal, because realized, >> that if all the spaces are local, then neither of them can be sync. >> So I banned is_sync + is_local options in the first commit. Did you >> remove it for the same reason? > > If I remember correctly,  your is_local check was not about local spaces, but > rather about whether the transaction was originating from the local instance. Yeah, also my bad. The name of my own variable confused me. > I separated txn_limbo behaviour judging by where the limbo_entries come from. > If they come from txn_commit_async(), this means this is  a  replica, and txn_limbo > waits  for  confirm messages. If they come from txn_commit(), this is master, so > txn_limbo gathers acks. It'd be better to judge by your is_local flag, I  believe. Yeah, better rely on rows content than on async vs normal commit way. Both in txn_commit_async() and txn_commit(). > We probably should put a fixme here also.