Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Oleg Babin <olegrok@tarantool.org>,
	tarantool-patches@dev.tarantool.org,
	yaroslav.dynnikov@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH vshard 6/6] router: update master using a hint from storage
Date: Fri, 2 Jul 2021 23:36:16 +0200
Message-ID: <6073e325-a77b-c5c1-8950-950f41c36ac3@tarantool.org> (raw)
In-Reply-To: <2dc3ff07-5f6e-294f-670a-5dcc5948c839@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.

  reply	other threads:[~2021-07-02 21:36 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 22:09 [Tarantool-patches] [PATCH vshard 0/6] Master discovery Vladislav Shpilevoy via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 1/6] replicaset: introduce netbox_wait_connected() Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:46   ` Oleg Babin via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 2/6] test: sort some table prints Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:46   ` Oleg Babin via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 3/6] storage: introduce vshard.storage._call('info') Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:46   ` Oleg Babin via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 4/6] config: introduce master 'auto' replicaset option Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:47   ` Oleg Babin via Tarantool-patches
2021-07-02 21:32     ` Vladislav Shpilevoy via Tarantool-patches
2021-07-05  9:23       ` Oleg Babin via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 5/6] router: introduce automatic master discovery Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:48   ` Oleg Babin via Tarantool-patches
2021-07-02 21:35     ` Vladislav Shpilevoy via Tarantool-patches
2021-07-05  9:24       ` Oleg Babin via Tarantool-patches
2021-07-05 20:53         ` Vladislav Shpilevoy via Tarantool-patches
2021-07-06  8:54           ` Oleg Babin via Tarantool-patches
2021-07-06 21:19             ` Vladislav Shpilevoy via Tarantool-patches
2021-07-01 22:09 ` [Tarantool-patches] [PATCH vshard 6/6] router: update master using a hint from storage Vladislav Shpilevoy via Tarantool-patches
2021-07-02 11:49   ` Oleg Babin via Tarantool-patches
2021-07-02 21:36     ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-07-05  9:24       ` Oleg Babin via Tarantool-patches
2021-07-05 20:53         ` Vladislav Shpilevoy via Tarantool-patches
2021-07-06  8:55           ` Oleg Babin via Tarantool-patches
2021-07-02 21:36 ` [Tarantool-patches] [PATCH vshard 7/6] util: truncate too long fiber name Vladislav Shpilevoy via Tarantool-patches
2021-07-05  9:23   ` Oleg Babin via Tarantool-patches
2021-08-03 21:55 ` [Tarantool-patches] [PATCH vshard 0/6] Master discovery Vladislav Shpilevoy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6073e325-a77b-c5c1-8950-950f41c36ac3@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=yaroslav.dynnikov@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git