[Tarantool-patches] [PATCH v2 1/2] box: refactor in_promote using a guard

[Serge Petrenko]

26.05.2021 10:25, Cyrill Gorcunov пишет:
> On Tue, May 25, 2021 at 01:39:28PM +0300, Serge Petrenko wrote:
>> ---
>>   src/box/box.cc | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/box/box.cc b/src/box/box.cc
>> index c10e0d8bf..894e3d0f4 100644
>> --- a/src/box/box.cc
>> +++ b/src/box/box.cc
>> @@ -1562,6 +1562,9 @@ box_promote(void)
>>   	int rc = 0;
>>   	int quorum = replication_synchro_quorum;
>>   	in_promote = true;
>> +	auto promote_guard = make_scoped_guard([&] {
>> +		in_promote = false;
>> +	});
> Looks ok to me, though I must confess I always consider such
> flags spread all over the code is somehow clumsy. Since this
> is a common pattern in our cpp code lets keep it but still in
> my humble opinion we could rather move all box_promote code
> into some box_do_promote helper and we would have
>
> int
> box_promote(void)
> {
> 	static bool in_promote = false;
> 	if (in_promote) {
> 		diag_set(ClientError, ER_UNSUPPORTED, "box.ctl.promote",
> 			 "simultaneous invocations");
> 		return -1;
> 	}
>
> 	in_promote = true;
> 	int rc = box_do_promote();
> 	in_promote = false;
>
> 	return rc;
> }
>
> but surely this is not a request for code refactoring, current form
> is ok as well ;)
>
> Ack.
>
> Serge, while you're at this code anyway, could you please change
>
> 	switch (box_election_mode) {
> 	case ELECTION_MODE_OFF:
> 		try_wait = true;
> 		break;
> 	...
> 	default:
> 		panic("enexpected box_election_mode mode");
> 		break;
> 	}
>
> instead of unreacheable() call. We should stop using unreacheable()
> as much as we could.

Thanks for the review!
I think this piece of code is unrelated to what I'm changing.
This commit was introduced to avoid multiple additions
of `in_promote = false;` in the next commit.

The change you propose is good, but it's out of scope of this patchset IMO.
I may change apply it though if you insist.

-- 
Serge Petrenko