[TNTP-8909] get rid of retry on bucket ref error by Satbek · Pull Request #511 · tarantool/crud

Satbek · 2026-08-18T14:10:17Z

During rebalancing the source storage rejects a request with a bucket
ref error (e.g. TRANSFER_IS_IN_PROGRESS) and reports the transfer
destination. The router followed this redirect and retried the request
directly on the destination.

The destination, however, may not have received the bucket yet: it has
neither the _bucket record nor the data, and it is still in fast mode,
so no bucket ownership check is performed. The request was applied
against an empty space and acknowledged to the client. Once
bucket_recv() delivered the data, the cluster state diverged from the
acknowledged result: a deleted tuple reappeared, and the final result
of replace/update/upsert could differ from what was acknowledged.

Now bucket ref errors are returned to the caller without a retry. The
router only resets the bucket route cache, so the next request
re-discovers the bucket location via vshard discovery.

Closes #509

During rebalancing the source storage rejects a request with a bucket ref error (e.g. TRANSFER_IS_IN_PROGRESS) and reports the transfer destination. The router followed this redirect and retried the request directly on the destination. The destination, however, may not have received the bucket yet: it has neither the _bucket record nor the data, and it is still in fast mode, so no bucket ownership check is performed. The request was applied against an empty space and acknowledged to the client. Once bucket_recv() delivered the data, the cluster state diverged from the acknowledged result: a deleted tuple reappeared, and the final result of replace/update/upsert could differ from what was acknowledged. Now bucket ref errors are returned to the caller without a retry. The router only resets the bucket route cache, so the next request re-discovers the bucket location via vshard discovery. Closes #509

Safe mode is enabled by an on_replace trigger on _bucket. The SENDING/RECEIVING rows replicate and fire the trigger on replicas too, so after a failover the promoted replica is still in safe mode and keeps rejecting writes to the buckets being transferred.

Serpentian

Great work!

Serpentian · 2026-08-20T10:05:11Z

@@ -85,9 +85,9 @@ local function wrap_vshard_err(vshard_router, err, func_name, replicaset_id, buc
 end


Since vshard 0.1.41 a transfer no longer starts at SENDING, it's READONLY now, which crud completely ignores. I'd fix it, while we're here, the safe mode should be enabled also on UPDATE to READONLY.

We should not drop the SENDING for compatibility with old vshard versions, IMHO.

P.S. VShard itself uses that status to wait for all rw refs to end and in order to check, that on replicas rw refs are also ended. In the future it's possible, that vshard will use that status in order to make arbitrary bucket READONLY, and crud should respect that.

thanks for information about READONLY status

added additional condition (update + READONLY) in trigger

Serpentian · 2026-08-20T10:07:54Z

+-- so safe mode must survive a failover: a promoted replica stays in safe
+-- mode.
+--
+local pgroup_safe_mode_failover = t.group('safe_mode_failover', {


Why the whole new cluster for the single case? Can't it reuse the pgroup_transfer_gap (maybe just rename it to smth more general)

Serpentian · 2026-08-20T10:13:56Z

 --- Executes a vshard call and retries once after performing recovery actions
--- like bucket cache reset, destination redirect (for single calls), or master discovery.
+--- like bucket cache reset or master discovery.
 local function call_with_retry_and_recovery(vshard_router,


Why do we need the retry now, when no "recovery actions" (as in the comment above, which is btw not correct) are performed. The search for the leader is anyway done inside the rs:call* before the call itself, so one more search for it won't help anyway.

So, maybe we can just completely get rid of retry?

Serpentian · 2026-08-20T10:29:20Z

+        -- received the bucket yet and would apply the request against an
+        -- empty space in fast mode (gh-509). The next request re-discovers
+        -- the bucket location via the router.
+        return resp, err


And if we do want to retry in that function, we can do that, but it should be done with bucket_reset + new_rs = router:route(bid) + new_rs:call.

And why do we start exiting on all of the ref errors: e.g. there NON_MASTER error, we can safely retry it even without bucket reset. If there's destination in the , then for retry reset + route.

So, it seems to me, we should either retry or not in that function, now it looks messy to me: the retry is present, but it's meaningless, or am I missing smth?

Serpentian · 2026-08-20T10:57:46Z

-        func_name, func_args, {timeout = timeout, request_timeout = request_timeout}, true)
+        func_name, func_args, {timeout = timeout, request_timeout = request_timeout})
    if err ~= nil then
        return nil, wrap_vshard_err(vshard_router, err, func_name, nil, bucket_id)


This wrap_vshard_err function internally does router:route() in order to create error object, but the call_with_retry_and_discovery uses the bucket_reset. So the wrap_vshard_err will mention replicaset_idof another replicaset and not the one, we did request to

Serpentian · 2026-08-20T12:41:34Z

+
+local pgroup_transfer_gap = t.group('double_buckets_transfer_gap', {
+    {
+        backend = helpers.backend.CONFIG,


It seams, we cannot hardcode the config backend in the test group, the same comment for other commit. The other groups in this file get their backends from helpers.backend_matrix(), which adds helpers.backend.CONFIG only if helpers.is_tarantool3_crud_roles_supported()

Serpentian · 2026-08-20T13:08:56Z

+    end)
+    source:exec(function()
+        local t = require('luatest')
+        t.assert(_G.send_fiber:join())


Nit: this checks, that the fiber was joined successfully, not that the send succeeded

Serpentian · 2026-08-20T13:10:03Z

+    helpers.stop_cluster(g.cluster, g.params.backend)
+end)
+
+pgroup_safe_mode_failover.test_safe_mode_survives_failover = function(g)


Why the test (and the group) are named as failover, when master is not changed in the test

Serpentian · 2026-08-20T13:24:42Z

+    local source_replica = g.cluster:server('s1-replica')
+    local destination = g.cluster:server('s2-master')
+
+    -- -- bucket_send requires both masters to be synced with their replicas


Double comment marker, and below too

Serpentian · 2026-08-20T13:26:51Z

+-- CRUD retries there directly. The destination must not accept that retry
+-- before bucket_recv() creates the bucket as RECEIVING.
+--
+pgroup_transfer_gap.test_delete_before_bucket_receive = function(g)


If it's not difficult, can we also reproduce the issue with faliover and instance restart to be sure, that the issue is resolved. And add these cases to tests. Up to u

Since vshard 0.1.41 a bucket transfer no longer starts in the SENDING status, it starts in READONLY instead. Crud ignored it and enabled the safe mode only when the bucket reached SENDING, i.e. later than the transfer actually started. The safe mode is now also enabled on UPDATE to READONLY. The SENDING condition is kept for compatibility with older vshard versions.

Satbek force-pushed the tntp-8909-double-buckets branch from 077fa83 to e8ec406 Compare August 19, 2026 08:07

Satbek requested review from Serpentian, a1div0 and vakhov August 19, 2026 10:15

Satbek added 2 commits August 19, 2026 15:50

Satbek force-pushed the tntp-8909-double-buckets branch from e8ec406 to 0b500ca Compare August 19, 2026 10:50

a1div0 approved these changes Aug 19, 2026

View reviewed changes

vakhov approved these changes Aug 19, 2026

View reviewed changes

Serpentian reviewed Aug 20, 2026

View reviewed changes

Serpentian assigned Satbek Aug 20, 2026

Satbek added 2 commits August 21, 2026 13:07

test: review fixes

39b621f

Satbek force-pushed the tntp-8909-double-buckets branch from 3eddbdf to 39b621f Compare August 21, 2026 15:10

Navigation Menu

Uh oh!

[TNTP-8909] get rid of retry on bucket ref error - #511

[TNTP-8909] get rid of retry on bucket ref error#511
Satbek wants to merge 4 commits into
masterfrom
tntp-8909-double-buckets

Satbek commented Aug 18, 2026 •

edited

Loading

Uh oh!

Serpentian left a comment

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Satbek Aug 21, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Serpentian Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -85,9 +85,9 @@ local function wrap_vshard_err(vshard_router, err, func_name, replicaset_id, buc
		end

Uh oh!

Conversation

Satbek commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Serpentian 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Satbek commented Aug 18, 2026 •

edited

Loading