TNTP-2109: Switch to "safe" mode on vshard rebalance #462

ita-sammann · 2025-11-19T08:19:30Z

What has been done? Why? What problem is being solved?

I didn't forget about

Tests
Changelog
Documentation

Closes #???

Satbek

What if, instead of threading bucket_id through all the call chains just to support safe_mode, we add:

bucket_ref(tuple.bucket_id)
bucket_unref(tuple.bucket_id)

directly in all write methods?

In “fast” mode bucket_ref / bucket_unref would be no-ops.
In “slow/safe” mode they would delegate to vshard.storage.bucket_ref / vshard.storage.bucket_unref.

This way:

the call_on_storage signature doesn’t have to change (no need to pass bucket_id around);
ref/unref calls live exactly where they logically belong (next to writes), instead of being scattered across different places only for safe_mode.

To me this looks more readable and less error-prone than propagating bucket_id solely for safe mode handling.

Also we need to add document about rebalance in README or another document in doc dir

Satbek · 2025-11-19T11:06:16Z

crud/common/map_call_cases/base_iter.lua

    self.next_index, self.next_replicaset = next(self.replicasets, self.next_index)

-    return self.func_args, replicaset, replicaset_id
+    return self.func_args, replicaset, replicaset_id, {}


please write 4th return name in doc for method

Satbek · 2025-11-19T11:10:37Z

crud/common/map_call_cases/batch_insert_iter.lua

    self.next_index, self.next_batch = next(self.batches_by_replicasets, self.next_index)

-    return func_args, replicaset, replicaset_id
+    return func_args, replicaset, replicaset_id, bucket_ids


please write 4th return name in doc for method

Satbek · 2025-11-19T11:10:57Z

crud/common/map_call_cases/batch_upsert_iter.lua

    self.next_index, self.next_batch = next(self.batches_by_replicasets, self.next_index)

-    return func_args, replicaset, replicaset_id
+    return func_args, replicaset, replicaset_id, bucket_ids


please write 4th return name in doc for method

Satbek · 2025-11-19T11:23:01Z

crud/common/rebalance.lua

+local SETTINGS_SPACE_NAME = '_crud_settings'
+
+
+local M = rawget(_G, MODULE_INTERNALS)


there is no rawset for MODULE_INTERNALS

may be just

local M = { safe_mode = false, safe_mode_enable_hooks = {}, safe_mode_disable_hooks = {}, _router_cache_last_clear_ts = fiber.time() }

Satbek · 2025-11-19T11:28:33Z

crud/common/rebalance.lua

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


may be add constant for _crud.safe_mode_enable too?

Added a constant

Satbek · 2025-11-19T12:12:51Z

crud/common/rebalance.lua

+    local stored_safe_mode
+    if box.space[SETTINGS_SPACE_NAME] == nil then
+        create_space()
+        box.space[SETTINGS_SPACE_NAME]:insert{ 'safe_mode', false }
+    else
+        stored_safe_mode = box.space[SETTINGS_SPACE_NAME]:get{ 'safe_mode' }
+    end
+    M.safe_mode = stored_safe_mode and stored_safe_mode.value or false
+
+    if M.safe_mode then
+        for hook, _ in pairs(M.safe_mode_enable_hooks) do
+            hook()
+        end
+    else
+        box.space._bucket:on_replace(safe_mode_trigger)
+        for hook, _ in pairs(M.safe_mode_disable_hooks) do
+            hook()
+        end
+    end


may be move all this logic to box.watch(box.status)?

Yeah, you're probably right. Anyway the node will not get any requests before box.status event.

Satbek · 2025-11-19T12:53:53Z

crud/common/call.lua

+       err.name == 'BUCKET_IS_LOCKED' or
+       err.name == 'TRANSFER_IS_IN_PROGRESS' then


indentations

I just tried to beautify it)

Satbek · 2025-11-19T13:10:58Z

crud/common/call.lua

+        vshard_router:_bucket_reset(err.bucket_id)
+
+        -- Substitute replicaset only for single bucket_id calls.
+        if err.destination and vshard_router.replicasets[err.destination] and #bucket_ids == 1 then
+            replicaset = vshard_router.replicasets[err.destination]
+        else
+            return nil, err
+        end


from here?

https://github.com/tarantool/vshard/blob/1f4c2ccfad6419882d1801d85f2b801f7b54b813/vshard/router/init.lua#L678-L713

Yes, but very very simplified. Mostly because bucket_set() can't be accessed from outside vshard.

Added a comment about vshard above

Satbek · 2025-11-19T13:51:30Z

crud/common/sharding/init.lua

        local record_by_replicaset = batches[replicaset_id] or {
            replicaset = replicaset,
            tuples = {},
+            bucket_ids = {},


on write requests crud calculates bucket_Ids , so it can be retrieved from data

Satbek · 2025-11-19T13:59:52Z

crud/common/call.lua

+end
+
+local function call_on_storage_safe(run_as_user, bucket_ids, mode, func_name, ...)
+    fiber.name(CRUD_CALL_FIBER_NAME .. func_name)


should we mark safe functions?

It is definitely not necessary but may be useful for debugging

Satbek · 2025-11-20T05:46:12Z

also switches between safe/fast mode must be logged

ita-sammann added 6 commits October 28, 2025 17:24

TNTP-2109: Implement slow mode for single calls

67260d0

TNTP-2109: Add bucket_ref to map calls

9b79a68

Fix luachecks warnings

7730a5c

TNTP-2109: 'Fast' mode without any fiber manipulations

8898804

TNTP-2109: 'Fast' mode with fiber renaming

358d0ac

TNTP-2109: Auto switch to safe mode on rebalance start

475590a

ita-sammann requested a review from Satbek November 19, 2025 08:19

TNTP-2109: Process bucket_ref errors in crud.router

90e1fa2

ita-sammann force-pushed the TNTP-2109-safe-mode-switch branch from 38f6747 to 90e1fa2 Compare November 19, 2025 10:37

Satbek reviewed Nov 19, 2025

View reviewed changes

TNTP-2109: Multiple minor fixes

1fa88c7

ita-sammann added 2 commits November 20, 2025 15:27

TNTP-2109: Add checks for safe mode trigger and stored value

eefef58

TNTP-2109: Check that tarantool supports box.watch

5f3f46f

ita-sammann force-pushed the TNTP-2109-safe-mode-switch branch from 22a0fcc to 5f3f46f Compare November 20, 2025 14:22

TNTP-2109: Add rebalance related metrics

339ca8f

ita-sammann requested a review from DifferentialOrange November 25, 2025 15:47

ita-sammann added 2 commits November 25, 2025 21:32

TNTP-2109: Add safe mode on/off to test matrix

0c6370f

TNTP-2109: Do not disable safe mode on server stop

6683748

		local SETTINGS_SPACE_NAME = '_crud_settings'


		local M = rawget(_G, MODULE_INTERNALS)

		err.name == 'BUCKET_IS_LOCKED' or
		err.name == 'TRANSFER_IS_IN_PROGRESS' then

TNTP-2109: Switch to "safe" mode on vshard rebalance #462

Are you sure you want to change the base?

TNTP-2109: Switch to "safe" mode on vshard rebalance #462

Uh oh!

Conversation

ita-sammann commented Nov 19, 2025

Uh oh!

Satbek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

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!

Satbek commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Satbek left a comment •

edited

Loading