[TNTP-2109] Switch to safe mode on vshard rebalance #467

ita-sammann · 2025-12-05T10:59:56Z

Implement "safe" mode to prevent writing data to wrong replicaset when vshard rebalance is in progress.
Auto switch to safe mode when rebalance process starts.
Manual return to fast mode.

I didn't forget about

Tests
Changelog
Documentation

Co-authored-by: Satbek Turganbayev <s.turganbayev@corp.mail.ru>

In fast mode ref/unref do nothing. In safe mode they call bucket_refro,bucket_refrw, bucket_unrefo,bucket_unrefrw. Also added ref error handle from storages. On ref error router will reset bucket, change replicaset for single operations and retry request.

* add access to _bucket space in privillage tests * fix select_readview_test * enable double buckets test * add safe_mode_disable for cartirdge reload test: test_storage * add safe_mode_disable for test_any_vshard_call_timeout

Satbek · 2025-12-08T07:02:54Z

crud/common/sharding/bucket_ref_unref.lua

+--- on fast mode it does nothing.
+--- default is fast mode.
+
+--- bucket_refw and bucket_unrefrw must be called in one transaction in order to prevent


Suggested change

--- bucket_refw and bucket_unrefrw must be called in one transaction in order to prevent

--- bucket_ref/bucket_unref must be called in one transaction in order to prevent

Satbek · 2025-12-08T07:06:17Z

crud/common/sharding/bucket_ref_unref.lua

+--- set safe mode func
+--- from rebalance.safe_mode_status
+function M.set_safe_mode_status(safe_mode_status)
+    M.safe_mode_status = safe_mode_status


may be this can be removed because there are rebalance.register_safe_mode_enable_hook, rebalance.register_safe_mode_disable_hook functions

Satbek · 2025-12-08T07:09:04Z

crud/common/call.lua

+    call_on_storage = call_on_storage_safe
+
+    for fb_id, fb in pairs(fiber.info()) do
+        if string.find(fb.name, CRUD_CALL_FUNC_NAME .. '/fast/') then


string.startswith might be faster, or even

if safe_mode_allowed_fast_methods[fb.name] == true then

Satbek · 2025-12-08T07:34:27Z

crud/common/rebalance.lua

+local function safe_mode_enable()
+    if not box.info.ro then
+        box.space[SETTINGS_SPACE_NAME]:replace{ 'safe_mode', true }
+        for _, trig in pairs(box.space._bucket:on_replace()) do
+            if trig == safe_mode_trigger then
+                box.space._bucket:on_replace(nil, safe_mode_trigger)
+            end
+        end
+    end
+    M.safe_mode = true
+
+    for hook, _ in pairs(M.safe_mode_enable_hooks) do
+        hook()
+    end
+
+    log.info('Rebalance safe mode enabled')
+end
+
+local function safe_mode_disable()
+    if not box.info.ro then
+        box.space[SETTINGS_SPACE_NAME]:replace{ 'safe_mode', false }
+        box.space._bucket:on_replace(safe_mode_trigger)
+    end
+    M.safe_mode = false
+
+    for hook, _ in pairs(M.safe_mode_disable_hooks) do
+        hook()
+    end
+
+    log.info('Rebalance safe mode disabled')
+end


may be add some comments on trigger add and delete. Why we adding and deleting it.
Someone might decide that the trigger needs to be enabled in safe_mode_enable.

Satbek · 2025-12-08T07:42:41Z

crud/common/rebalance.lua

+M.router = {
+    cache_clear = router_cache_clear,
+    cache_length = router_cache_length,
+    cache_last_clear_ts = router_cache_last_clear_ts,
+}


we need tests for cache_clear scenario

Satbek · 2025-12-08T07:52:44Z

test/integration/double_buckets_test.lua

+pgroup_duplicates.test_duplicates = function(g)
+    t.skip_if(
+        not (
+            utils.tarantool_version_at_least(3, 1) and (g.params.backend == "config")


todo: implement test for other backend types. 2.11 and cartridge

Satbek · 2025-12-08T07:53:34Z

test/integration/double_buckets_test.lua

+    },
+    get = {
+        call = function (g, key)
+            return g.router:call('crud.get', { 'customers', key })


mode='write' in options. to make call to the master

Satbek · 2025-12-08T07:56:06Z

test/helper.lua

        end
    end
+
+    if g.params and g.params.safe_mode ~= nil then


I think we should check bootstrap finish (etalon buckets count on storages, with active statuses) before applying fast, slow mode.

vakhov

Thanks for the PR! I’ve gone through the changes and left a few small questions.

vakhov · 2025-12-08T07:53:20Z

crud/common/call.lua

+        if string.find(fb.name, CRUD_CALL_FUNC_NAME .. '/fast/') then
+            local do_kill = true
+            for _, allowed_method in ipairs(safe_mode_allowed_fast_methods) do
+                if fb.name == allowed_method then
+                    do_kill = false
+                    break
+                end
+            end
+            if do_kill then
+                fiber.kill(fb_id)
+            end


Maybe we could make this check a bit stricter, e.g. by checking for a prefix (something like fb.name:startswith(CRUD_CALL_FIBER_NAME .. '/fast/')), to minimize the risk of accidental matches? We could also adding a small debug log with the number of killed fibers to make debugging.

vakhov · 2025-12-08T07:57:07Z

crud/common/rebalance.lua

+    if (op == 'INSERT' and new.status == vshard_consts.BUCKET.RECEIVING) or
+            (op == 'REPLACE' and new.status == vshard_consts.BUCKET.SENDING) then
+        box.broadcast(SAFE_MOD_ENABLE_EVENT, true)


Safe mode is enabled only on RECEIVING/SENDING. Could there be other transitions we should handle too? Could there be a situation where a bucket is initially created in SENDING or goes through other statuses that we don’t catch here?

vakhov · 2025-12-08T08:00:39Z

crud/common/sharding/bucket_ref_unref.lua

+    local err = M.BucketRefError:new(M.BucketRefError:new(
+        "failed bucket_ref: %s, bucket_id: %s",
+        vshard_ref_err.name,
+        bucket_id
+    ))


It looks like a single call would be enough:

Suggested change

local err = M.BucketRefError:new(M.BucketRefError:new(

"failed bucket_ref: %s, bucket_id: %s",

vshard_ref_err.name,

bucket_id

))

local err = M.BucketRefError:new(

"failed bucket_ref: %s, bucket_id: %s",

vshard_ref_err.name,

bucket_id

)

vakhov · 2025-12-08T08:03:44Z

crud/common/sharding/bucket_ref_unref.lua

+        local ref_ok, bucket_refrw_err = safe_methods.bucket_refrw(bucket_id)
+        if not ref_ok then
+
+            table.insert(bucket_ref_errs, bucket_refrw_err.bucket_ref_errs[1])


If the inner error has no bucket_ref_errs, this will hit a nil index. Maybe worth guarding?

ita-sammann and others added 5 commits December 5, 2025 13:42

[TNTP-2109] Switch to safe mode on vshard rebalance

2f630af

[TNTP-2109] Add safe mode with bucket_ref/unref

9b626bb

[TNTP-2109] Add tests for safe mode

abbcfbd

Co-authored-by: Satbek Turganbayev <s.turganbayev@corp.mail.ru>

Fix tests after fast/safe mode changes

8ea36f9

* add access to _bucket space in privillage tests * fix select_readview_test * enable double buckets test * add safe_mode_disable for cartirdge reload test: test_storage * add safe_mode_disable for test_any_vshard_call_timeout

ita-sammann mentioned this pull request Dec 5, 2025

TNTP-2109: call bucket_ref/bucket_unref on crud operations #463

Closed

3 tasks

ita-sammann requested a review from Satbek December 5, 2025 11:31

ita-sammann marked this pull request as ready for review December 5, 2025 13:38

ita-sammann requested review from DifferentialOrange, a1div0, p0rtale and vakhov December 5, 2025 13:38

Allow some fast methods to finish after rebalance started

a8352f2

Satbek requested changes Dec 8, 2025

View reviewed changes

vakhov reviewed Dec 8, 2025

View reviewed changes

	--- bucket_refw and bucket_unrefrw must be called in one transaction in order to prevent
	--- bucket_ref/bucket_unref must be called in one transaction in order to prevent

[TNTP-2109] Switch to safe mode on vshard rebalance #467

Are you sure you want to change the base?

[TNTP-2109] Switch to safe mode on vshard rebalance #467

Uh oh!

Conversation

ita-sammann commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vakhov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ita-sammann commented Dec 5, 2025 •

edited

Loading