From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 CD1B9469719 for ; Mon, 7 Sep 2020 18:45:46 +0300 (MSK) Date: Mon, 7 Sep 2020 18:45:44 +0300 From: Sergey Ostanevich Message-ID: <20200907154544.GA46@tarantool.org> References: <78668e078bb8298ad2b88b029fb9d0c5f475956d.1599173312.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <78668e078bb8298ad2b88b029fb9d0c5f475956d.1599173312.git.v.shpilevoy@tarantool.org> 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: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch, just one comment below. Regards, Sergos On 04 сен 00:51, Vladislav Shpilevoy wrote: > Struct replicaset didn't store a number of registered replicas. > Only an array, which was necessary to fullscan each time when want > to find the count. > > That is going to be needed in Raft to calculate election quorum. > The patch makes the count tracked so as it could be found for > constant time by simply reading an integer. > > Needed for #1146 > --- > src/box/replication.cc | 3 +++ > src/box/replication.h | 7 +++++++ > 2 files changed, 10 insertions(+) > > 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() > > say_info("assigned id %d to replica %s", > replica->id, tt_uuid_str(&replica->uuid)); > @@ -267,6 +268,8 @@ replica_clear_id(struct replica *replica) > * replication. > */ > replicaset.replica_by_id[replica->id] = NULL; > + assert(replicaset.size > 0); > + --replicaset.size; > if (replica->id == instance_id) { > /* See replica_check_id(). */ > assert(replicaset.is_joining); > diff --git a/src/box/replication.h b/src/box/replication.h > index ddc2bddf4..69cc820c9 100644 > --- a/src/box/replication.h > +++ b/src/box/replication.h > @@ -217,6 +217,13 @@ struct replicaset { > bool is_joining; > /* A number of anonymous replicas following this instance. */ > int anon_count; > + /** > + * Number of registered replicas. That includes all of them - connected, > + * disconnected, connected not directly, just present in _cluster. If an > + * instance has an ID, has the same replicaset UUID, then it is > + * accounted here. > + */ > + int size; > /** Applier state. */ > struct { > /** > -- > 2.21.1 (Apple Git-122.3) >