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 B0DCB469719 for ; Tue, 8 Sep 2020 01:54:52 +0300 (MSK) References: <78668e078bb8298ad2b88b029fb9d0c5f475956d.1599173312.git.v.shpilevoy@tarantool.org> <20200907154544.GA46@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 8 Sep 2020 00:54:51 +0200 MIME-Version: 1.0 In-Reply-To: <20200907154544.GA46@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 04/10] replication: track registered replica count List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review! >> diff --git a/src/box/replication.cc b/src/box/replication.cc >> index ef0e2411d..20f16206a 100644 >> --- a/src/box/replication.cc >> +++ b/src/box/replication.cc >> @@ -247,6 +247,7 @@ replica_set_id(struct replica *replica, uint32_t replica_id) >> tt_uuid_str(&replica->uuid)); >> } >> replicaset.replica_by_id[replica_id] = replica; >> + ++replicaset.size; > > There's another use of the replica_set_id() inside the register_replica() > of alter.cc. Apparently, it's just for re-assign the ID without adding of > a new node. I would propose to move this counter into replicaset_add() A replica can become anon, and can be deleted. So I can't manage the counter only in replicaset_add. It should decrease somewhere. Talking of alter.cc, instance ID is a primary key of _cluster. So it can't be changed for an existing replica. It needs to be deleted and added with a new ID. It means, instance ID is essentially a read-only field. Also this is validated in replica_set_id() - it checks the replica wasn't registered before in an assertion - and in replica_clear_id() - it checks the replica is registered. There are no cases, when an ID is updated for an existing replica.