This is an automated email from the ASF dual-hosted git repository.

git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 4f14f65eb fix(cluster): stop raw key-value slot migration promptly 
when canceled (#3714)
4f14f65eb is described below

commit 4f14f65eb4180dcea15e11ea3831a6df4765ee87
Author: jonas <[email protected]>
AuthorDate: Mon Oct 26 09:33:36 2026 +0800

    fix(cluster): stop raw key-value slot migration promptly when canceled 
(#3714)
    
    ### What problem does this PR solve?
    
    Issue Number: none
    
    Kvrocks sets `stop_migration_` to `true` on the source node when:
    
    - `FLUSHDB` or `FLUSHALL` is executed
    - the node role changes from master to slave
    - `Server::PrepareRestoreDB` waits for the migration to finish before
    closing and reopening RocksDB
    - the server shuts down and destroys the `SlotMigrator`
    
    This flag stops the current migration task without destroying the slot
    migration thread.
    
    The former redis-command migration path checked this flag while
    iterating keys, subkeys, sending command pipelines, and syncing WAL. The
    raw key-value path only checks it in `finishSuccessfulMigration`.
    
    That prevents a canceled task from being marked as successfully
    migrated, but it cannot stop the in-flight data transfer promptly, which
    leads to the following problems:
    
    1. Snapshot and WAL data keep being transferred after a flush or a role
    change.
    2. `PrepareRestoreDB` keeps waiting until the migration stage returns to
    `kNone`. For a large slot, RocksDB cannot be closed and reopened for a
    long time.
    3. After the migration has been canceled, `syncWAL` may still call
    `setForbiddenSlotRange`, acquiring the server exclusivity guard and
    forbidding writes to the migrating slot.
    4. When the migration is rate-limited, cancellation may not take effect
    until the whole snapshot and WAL transfer completes.
    
    ### What is changed and how it works?
    
    This PR adds `stop_migration_` checks to the main loops of the raw
    key-value migration:
    
    - when iterating each metadata key in `sendSnapshot`
    - when iterating each subkey of complex data types in `sendSnapshot`
    - when iterating each WAL item in `migrateIncrementalDataByRawKV`
    - before calling `setForbiddenSlotRange` in `syncWAL`
    
    When `stop_migration_` is found to be `true`, the migration returns
    `errMigrationTaskCanceled`.
    
    The existing migration state machine turns this error into a failed
    migration and then:
    
    - sets the import status on the destination node to failed
    - clears the partially imported data on the destination node
    - releases the RocksDB snapshot
    - clears the forbidden slot range
    - resets the migration stage to `kNone`
    
    The comment in the FLUSHDB slot migration integration test is also
    updated to match the new cancellation behavior.
    
    ### AI Usage Disclosure
    
    AI tools(Kimi K3) were only used to analyze the code logic of the two
    migration paths and to draft this PR description. All code changes were
    made manually, and the final changes and technical conclusions have been
    reviewed and confirmed by the author.
    
    ---------
    
    Co-authored-by: Twice <[email protected]>
    Co-authored-by: hulk <[email protected]>
    Co-authored-by: shouyan <[email protected]>
---
 src/cluster/slot_migrate.cc                              | 14 ++++++++++++--
 tests/gocase/integration/slotmigrate/slotmigrate_test.go |  4 ++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/cluster/slot_migrate.cc b/src/cluster/slot_migrate.cc
index 66228058b..3e7d2b7a0 100644
--- a/src/cluster/slot_migrate.cc
+++ b/src/cluster/slot_migrate.cc
@@ -584,12 +584,14 @@ Status SlotMigrator::sendSnapshot() {
   BatchSender batch_sender(*dst_fd_, migrate_batch_size_bytes_, 
migrate_batch_bytes_per_sec_);
 
   for (iter.Seek(prefix); iter.Valid(); iter.Next()) {
+    if (stop_migration_) {
+      return {Status::NotOK, std::string(errMigrationTaskCanceled)};
+    }
     // Iteration is out of range
     auto key_slot_id = ExtractSlotId(iter.Key());
     if (!slot_range.Contains(key_slot_id)) {
       break;
     }
-
     auto redis_type = iter.Type();
     std::string log_data;
     if (redis_type == RedisType::kRedisList) {
@@ -617,6 +619,9 @@ Status SlotMigrator::sendSnapshot() {
     }
 
     for (subkey_iter->Seek(); subkey_iter->Valid(); subkey_iter->Next()) {
+      if (stop_migration_) {
+        return {Status::NotOK, std::string(errMigrationTaskCanceled)};
+      }
       GET_OR_RET(batch_sender.Put(subkey_iter->ColumnFamilyHandle(), 
subkey_iter->Key(), subkey_iter->Value()));
       if (batch_sender.IsFull()) {
         GET_OR_RET(sendMigrationBatch(&batch_sender));
@@ -671,7 +676,9 @@ Status SlotMigrator::syncWAL() {
     wal_begin_seq_ = wal_incremental_seq;
     epoch++;
   }
-
+  if (stop_migration_) {
+    return {Status::NotOK, std::string(errMigrationTaskCanceled)};
+  }
   setForbiddenSlotRange(slot_range_);
 
   wal_incremental_seq = storage_->GetDB()->GetLatestSequenceNumber();
@@ -708,6 +715,9 @@ Status SlotMigrator::migrateIncrementalDataByRawKV(uint64_t 
end_seq, BatchSender
   engine::WALIterator wal_iter(storage_, slot_range_);
   uint64_t start_seq = wal_begin_seq_ + 1;
   for (wal_iter.Seek(start_seq); wal_iter.Valid(); wal_iter.Next()) {
+    if (stop_migration_) {
+      return {Status::NotOK, std::string(errMigrationTaskCanceled)};
+    }
     if (wal_iter.NextSequenceNumber() > end_seq + 1) {
       break;
     }
diff --git a/tests/gocase/integration/slotmigrate/slotmigrate_test.go 
b/tests/gocase/integration/slotmigrate/slotmigrate_test.go
index 6c82bcf14..3934309ed 100644
--- a/tests/gocase/integration/slotmigrate/slotmigrate_test.go
+++ b/tests/gocase/integration/slotmigrate/slotmigrate_test.go
@@ -242,8 +242,8 @@ func TestSlotMigrateSourceServerFlushedOrKilled(t 
*testing.T) {
                require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate", 
slot, id1).Val())
                waitForMigrateState(t, rdb0, slot, SlotMigrationStateStarted)
                require.NoError(t, rdb0.FlushDB(ctx).Err())
-               // The stop flag set by FLUSHDB is only checked when finishing 
the migration,
-               // so it takes a while for the rate-limited migration to reach 
the failed state.
+               // The stop flag set by FLUSHDB is checked during the migration 
process,
+               // it takes a while for the rate-limited migration to reach the 
failed state.
                waitForMigrateStateInDuration(t, rdb0, slot, 
SlotMigrationStateFailed, time.Minute)
        })
 

Reply via email to