Re: [Tarantool-patches] [PATCH vshard 6/6] router: update master using a hint from storage

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the review!

>> diff --git a/vshard/error.lua b/vshard/error.lua
>> index e2d1a31..fa7bdaa 100644
>> --- a/vshard/error.lua
>> +++ b/vshard/error.lua
>> @@ -20,7 +20,7 @@ local error_message_template = {
>>       [2] = {
>>           name = 'NON_MASTER',
>>           msg = 'Replica %s is not a master for replicaset %s anymore',
>> -        args = {'replica_uuid', 'replicaset_uuid'}
>> +        args = {'replica_uuid', 'replicaset_uuid', 'master_uuid'}
> Error format string still contains only 2 arguments. Is it ok?

Yes. All the 'args' are remembered as the error object attributes.
I can use it to attach information which I don't want to add to
the message or simply can't.

Here I can't, because master_uuid might be nil. If it would be in
the format, it would be displayed as 'nil' sometimes, which would be
confusing.

>> diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua
>> index 7747258..660c786 100644
>> --- a/vshard/replicaset.lua
>> +++ b/vshard/replicaset.lua
>> @@ -570,6 +570,70 @@ local function rebind_replicasets(replicasets, old_replicasets)
>>       end
>>   end
>>   +--
>> +-- Let the replicaset know @a old_master_uuid is not a master anymore, should
>> +-- use @a candidate_uuid instead.
>> +-- Returns whether the request, which brought this information, can be retried.
>> +--
>> +local function replicaset_update_master(replicaset, old_master_uuid,
>> +                                        candidate_uuid)
>> +    local is_auto = replicaset.is_auto_master
>> +    local replicaset_uuid = replicaset.uuid
>> +    if old_master_uuid == candidate_uuid then
>> +        -- It should not happen ever, but be ready to everything.
>> +        log.warn('Replica %s in replicaset %s reports self as both master '..
>> +                 'and not master', master_uuid, replicaset_uuid)
>> +        return is_auto
>> +    end
>> +    local master = replicaset.master
>> +    if not master then
>> +        if not is_auto or not candidate_uuid then
>> +            return is_auto
>> +        end
>> +        local candidate = replicaset.replicas[candidate_uuid]
> 
> AFAIU it means that candidate_uuid doesn't belong to replicaset.
> 
> Why is it true?

It can happen if router config is different from the storage's due
to any reason. If the master is not in router's config, it won't be
able to find it by UUID.