From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <sergepetrenko@tarantool.org>
Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106])
 (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 4D958469719
 for <tarantool-patches@dev.tarantool.org>;
 Wed, 16 Sep 2020 13:59:44 +0300 (MSK)
References: <cover.1600124767.git.v.shpilevoy@tarantool.org>
 <fe070436c20efe6c6293e5a79c707fe3db3e848a.1600124767.git.v.shpilevoy@tarantool.org>
 <de13889b-b862-2fd6-3e05-b57466300c12@tarantool.org>
 <ce0e3e2c-aeb2-f033-1277-6ad4752e0225@tarantool.org>
From: Serge Petrenko <sergepetrenko@tarantool.org>
Message-ID: <57f360af-7964-b3e4-88fe-c9a45dfe5a19@tarantool.org>
Date: Wed, 16 Sep 2020 13:59:43 +0300
MIME-Version: 1.0
In-Reply-To: <ce0e3e2c-aeb2-f033-1277-6ad4752e0225@tarantool.org>
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit
Content-Language: ru
Subject: Re: [Tarantool-patches] [PATCH v2 3/4] replication: add is_anon
	flag to ballot
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org, gorcunov@gmail.com


16.09.2020 00:22, Vladislav Shpilevoy пишет:
> Thanks for the review!
>
>>> diff --git a/src/box/replication.cc b/src/box/replication.cc
>>> index ef0e2411d..8408f395e 100644
>>> --- a/src/box/replication.cc
>>> +++ b/src/box/replication.cc
>>> @@ -290,6 +290,8 @@ replica_clear_id(struct replica *replica)
>>>        }
>>>        if (replica_is_orphan(replica)) {
>>>            replica_hash_remove(&replicaset.hash, replica);
>>> +        replicaset.anon_count -= replica->anon;
>>> +        assert(replicaset.anon_count >= 0);
>> replica_clear_id() is only called on _cluster entry deletion.
>> So you're surely working with a normal replica, not anonymous.
>>
>> We may add an assset(!replica->anon) somewhere.
> Indeed:
>
> ====================
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index 8408f395e..c1fcdb660 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -290,8 +290,11 @@ replica_clear_id(struct replica *replica)
>   	}
>   	if (replica_is_orphan(replica)) {
>   		replica_hash_remove(&replicaset.hash, replica);
> -		replicaset.anon_count -= replica->anon;
> -		assert(replicaset.anon_count >= 0);
> +		/*
> +		 * The replica had an ID, it couldn't be anon by
> +		 * definition.
> +		 */
> +		assert(!replica->anon);
>   		replica_delete(replica);
>   	}
>   }
Thanks  for your answert!
Now LGTM.

-- 
Serge Petrenko