Tarantool development patches archive
 help / color / mirror / Atom feed
From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tarantool-patches@dev.tarantool.org,
	yaroslav.dynnikov@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 4/9] storage: bucket_recv() should check rs lock
Date: Wed, 10 Feb 2021 11:59:00 +0300	[thread overview]
Message-ID: <859c6dd6-3c84-f2ba-391b-49a669dc2d6b@tarantool.org> (raw)
In-Reply-To: <7cf081ec78f104b2a95918685733d823495a43ed.1612914070.git.v.shpilevoy@tarantool.org>

Thanks for your patch. LGTM.

On 10/02/2021 02:46, Vladislav Shpilevoy wrote:
> Locked replicaset (via config) should not allow any bucket moves
> from or to the replicaset.
>
> But the lock check was only done by bucket_send(). Bucket_recv()
> allowed to receive a bucket even if the replicaset is locked. The
> patch fixes it.
>
> It didn't affect automatic bucket sends, because lock is
> accounted by the rebalancer from the config. Only manual bucket
> moves could have this bug.
> ---
>   test/rebalancer/rebalancer_lock_and_pin.result   | 14 ++++++++++++++
>   test/rebalancer/rebalancer_lock_and_pin.test.lua |  4 ++++
>   vshard/storage/init.lua                          |  3 +++
>   3 files changed, 21 insertions(+)
>
> diff --git a/test/rebalancer/rebalancer_lock_and_pin.result b/test/rebalancer/rebalancer_lock_and_pin.result
> index 51dd36e..0bb4f45 100644
> --- a/test/rebalancer/rebalancer_lock_and_pin.result
> +++ b/test/rebalancer/rebalancer_lock_and_pin.result
> @@ -156,6 +156,20 @@ vshard.storage.bucket_send(1, util.replicasets[2])
>     message: Replicaset is locked
>     code: 19
>   ...
> +test_run:switch('box_2_a')
> +---
> +- true
> +...
> +-- Does not allow to receive either. Send from a non-locked replicaset to a
> +-- locked one fails.
> +vshard.storage.bucket_send(101, util.replicasets[1])
> +---
> +- null
> +- type: ShardingError
> +  code: 19
> +  name: REPLICASET_IS_LOCKED
> +  message: Replicaset is locked
> +...
>   --
>   -- Vshard ensures that if a replicaset is locked, then it will not
>   -- allow to change its bucket set even if a rebalancer does not
> diff --git a/test/rebalancer/rebalancer_lock_and_pin.test.lua b/test/rebalancer/rebalancer_lock_and_pin.test.lua
> index c3412c1..7b87004 100644
> --- a/test/rebalancer/rebalancer_lock_and_pin.test.lua
> +++ b/test/rebalancer/rebalancer_lock_and_pin.test.lua
> @@ -69,6 +69,10 @@ info.lock
>   -- explicitly.
>   --
>   vshard.storage.bucket_send(1, util.replicasets[2])
> +test_run:switch('box_2_a')
> +-- Does not allow to receive either. Send from a non-locked replicaset to a
> +-- locked one fails.
> +vshard.storage.bucket_send(101, util.replicasets[1])
>   
>   --
>   -- Vshard ensures that if a replicaset is locked, then it will not
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index c7335fc..298df71 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -995,6 +995,9 @@ local function bucket_recv_xc(bucket_id, from, data, opts)
>               return nil, lerror.vshard(lerror.code.WRONG_BUCKET, bucket_id, msg,
>                                         from)
>           end
> +        if is_this_replicaset_locked() then
> +            return nil, lerror.vshard(lerror.code.REPLICASET_IS_LOCKED)
> +        end
>           if not bucket_receiving_quota_add(-1) then
>               return nil, lerror.vshard(lerror.code.TOO_MANY_RECEIVING)
>           end

  reply	other threads:[~2021-02-10  8:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-09 23:46 [Tarantool-patches] [PATCH 0/9] VShard Map-Reduce, part 1, preparations Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 1/9] rlist: move rlist to a new module Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-11  6:50     ` Oleg Babin via Tarantool-patches
2021-02-12  0:09       ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 2/9] Use fiber.clock() instead of .time() everywhere Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 3/9] test: introduce a helper to wait for bucket GC Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:57   ` Oleg Babin via Tarantool-patches
2021-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 4/9] storage: bucket_recv() should check rs lock Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches [this message]
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 5/9] util: introduce yielding table functions Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches
2021-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 6/9] cfg: introduce 'deprecated option' feature Vladislav Shpilevoy via Tarantool-patches
2021-02-10  8:59   ` Oleg Babin via Tarantool-patches
2021-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 7/9] gc: introduce reactive garbage collector Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:00   ` Oleg Babin via Tarantool-patches
2021-02-10 22:35     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:50       ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 8/9] recovery: introduce reactive recovery Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:00   ` Oleg Babin via Tarantool-patches
2021-02-09 23:46 ` [Tarantool-patches] [PATCH 9/9] util: introduce binary heap data structure Vladislav Shpilevoy via Tarantool-patches
2021-02-10  9:01   ` Oleg Babin via Tarantool-patches
2021-02-10 22:36     ` Vladislav Shpilevoy via Tarantool-patches
2021-02-11  6:51       ` Oleg Babin via Tarantool-patches
2021-02-12  0:09         ` Vladislav Shpilevoy via Tarantool-patches
2021-03-05 22:03   ` Vladislav Shpilevoy via Tarantool-patches
2021-02-09 23:51 ` [Tarantool-patches] [PATCH 0/9] VShard Map-Reduce, part 1, preparations Vladislav Shpilevoy via Tarantool-patches
2021-02-12 11:02   ` Oleg Babin 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=859c6dd6-3c84-f2ba-391b-49a669dc2d6b@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=yaroslav.dynnikov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 4/9] storage: bucket_recv() should check rs lock' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox