From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (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 4DD80445320
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  6 Aug 2020 01:15:48 +0300 (MSK)
References: <0963ccd10f91b249116916d5c1c91f5eef11e438.1596575031.git.v.shpilevoy@tarantool.org>
 <CAK0MaD353JSXrA1VgyJg8ix0w+2EoU2DLVfZTa8eWDcdzCCj4A@mail.gmail.com>
 <CAK0MaD2AVy21HLjc=QAG7WG0xcqFAwXPy-CFsryjOOD4nrod4g@mail.gmail.com>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <e8c615ef-88b3-ed8d-d2a2-3c37029f46d1@tarantool.org>
Date: Thu, 6 Aug 2020 00:15:46 +0200
MIME-Version: 1.0
In-Reply-To: <CAK0MaD2AVy21HLjc=QAG7WG0xcqFAwXPy-CFsryjOOD4nrod4g@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [Tarantool-patches] [PATCH vshard 1/1] storage: allow replica
	to boot before master
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: Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org>
Cc: tml <tarantool-patches@dev.tarantool.org>

Thanks for the review!

>         diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
>         index c6a78fe..ed577f9 100644
>         --- a/vshard/storage/init.lua
>         +++ b/vshard/storage/init.lua
>         @@ -2435,8 +2446,11 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload)
>              local uri = luri.parse(this_replica.uri)
>              schema_upgrade(is_master, uri.login, uri.password)
> 
>         -    local old_trigger = box.space._bucket:on_replace()[1]
>         -    box.space._bucket:on_replace(bucket_generation_increment, old_trigger)
>         +    if is_master then
>         +        local old_trigger = M.bucket_on_replace
>         +        box.space._bucket:on_replace(bucket_generation_increment, old_trigger)
>         +        M.bucket_on_replace = bucket_generation_increment
>         +    end
> 
> 
> It seems that the trigger is never removed. If a replica was a master some time ago, it still has the trigger.
> Is it safe?

Yes, it is. The trigger only increments a counter in M. And the counter is
not used for anything out of the node. It should be just locally monotonic.
So it is ok to increment it even on replica. After all this is how it
worked before.

But again, after this comment I had second thoughts, and decided to remove the
trigger when switched to replica to be consistent. See v2 in a new mail thread.