Add Slot migration testing support - #15
Open
Fniakate8 wants to merge 6 commits into
Open
Conversation
Signed-off-by: Tracy <yuningt@amazon.com>
Signed-off-by: Tracy <yuningt@amazon.com>
Signed-off-by: Tracy <yuningt@amazon.com>
Signed-off-by: Tracy <yuningt@amazon.com>
Add ClusterTestCase.migrate_slot() so cluster tests can move a slot (and its keys) between live nodes without hand-writing the CLUSTER SETSLOT / GETKEYSINSLOT / MIGRATE protocol each time. - ClusterTestCase: migrate_slot (mark IMPORTING/MIGRATING, batch-move keys with MIGRATE KEYS, announce the new owner on every primary), get_slot_owner, wait_for_slot_owner (polls until all nodes agree, no fixed sleeps) - ClusterNodeHandle primitives: count_keys_in_slot, start_importing_slot, start_migrating_slot, assign_slot_owner, get_slot_owner_id, is_primary - Ownership is announced only on primaries (CLUSTER SETSLOT is rejected on replicas; they learn from their primary) - Multi-DB migration supported via dbs=...; requires cluster-databases > 1 - tests/test_slot_migration.py: single-key, many-key drain, multi-DB, caller-DB isolation, cluster-wide ownership, empty slot, and replica shards - README: slot migration subsection under the CME docs Built on the ClusterTestCase/ClusterNodeHandle helpers from valkey-io#12. Signed-off-by: Fanta Niakate <niakatf@amazon.com>
zackcam
reviewed
Aug 13, 2026
zackcam
left a comment
Collaborator
There was a problem hiding this comment.
Looks good to me at a high level overall!
| target = next(n for n in self.nodes if n.nodeid != source.nodeid) | ||
| return source, target | ||
|
|
||
| def test_migrate_slot_moves_key(self): |
Collaborator
There was a problem hiding this comment.
could maybe combine this and the test below mostly achieve the same purpose with the seocnd one being just more keys
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add slot migration testing support
This PR adds a helper so cluster tests can move a slot (and the keys in it) from one node to another on a live cluster, without hand-writing the actual
CLUSTER SETSLOT / GETKEYSINSLOT / MIGRATEprotocol each time.What's Added
ClusterTestCase.migrate_slot(source, target, slot, dbs=(0,)) — runs the manual migration protocol end to end: mark the slot IMPORTING on the target andMIGRATINGon the source, batch-move every key withMIGRATE ... KEYS, then announce the new owner on every primary.ClusterTestCase.get_slot_owner(slot)— the node that currently owns a slot.ClusterTestCase.wait_for_slot_owner(slot, owner) — polls until every node agrees on the new owner (no fixed sleeps).ClusterNodeHandle:count_keys_in_slot, start_importing_slot, start_migrating_slot, assign_slot_owner, get_slot_owner_id, is_primary.tests/test_slot_migration.py— 7 tests: single-key move, many-key drain (250 keys), multi-DB, caller-DB isolation, cluster-wide ownership, empty slot, andreplica shards.
Why this is needed
Slot migration is a core cluster operation (adding/removing nodes, rebalancing), but the framework has no helper for it — modules that want to test it hand-roll ~20 lines of low-level CLUSTER commands today (e.g. Search's
test_multidb_slot_migration_CME). This gives them one call and handles the fiddly parts (batch key move, draining slots with >100 keys, per-DB selection, primary-only ownership announcement).How to use
Once a slot's owner is known, migrating it is two calls:
self.migrate_slot(source_node, target_node, slot)# move slot + its keysself.wait_for_slot_owner(slot, target_node)# wait until the cluster agreesUse
self.get_slot_owner(slot)to find the current owner, andpass dbs=(0, 1, ...)to migrate keys across multiple databases. Migrating any DB other than 0, requires the cluster to be started withcluster-databases > 1(plain cluster mode only has DB 0).Design notes
SETSLOT/MIGRATEprotocol (what Search's real migration test uses today) rather than nativeCLUSTER MIGRATESLOTS, which has no current consumer.CLUSTER SETSLOTis rejected on replicas (they learn from their primary).Test plan