-
Notifications
You must be signed in to change notification settings - Fork 65
Add support of migrating with slot range #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
comitting this and will step backwards
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #304 +/- ##
============================================
+ Coverage 43.38% 46.11% +2.73%
============================================
Files 37 45 +8
Lines 2971 4341 +1370
============================================
+ Hits 1289 2002 +713
- Misses 1544 2131 +587
- Partials 138 208 +70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@bseto Thanks for your contribution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the migration functionality so that slot migration now works via slot ranges instead of individual slot IDs. The key changes include updating the MigrateSlotRequest type and its usage, refactoring various slot range functions (e.g., ParseSlotRange, AddSlotToSlotRanges, RemoveSlotFromSlotRanges), and adjusting associated tests and logging throughout the controller and client layers.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| store/slot_test.go | Updated tests for ParseSlotRange and slot range addition/removal logic |
| store/slot.go | Refactored slot range operations, including parsing and merging |
| store/cluster_shard.go | Updated Shard type to use pointer SlotRange for migrating slot |
| store/cluster_node.go | Changed MigrateSlot signature to use SlotRange |
| store/cluster.go | Updated shard lookup and migration logic to work with SlotRange |
| server/api/.go & controller/.go | Adjusted API endpoints, logging, and tests to operate on slot ranges |
| cmd/client/command/*.go | Modified CLI migration command to use string slot range input |
| consts/errors.go | Added error definitions relevant to new slot range migration |
|
@bseto Thank you! Looks good to me. I will give a shot on my side. |
Co-authored-by: hulk <hulk.website@gmail.com>
|
@bseto Thank you! |
No problem! |
This PR addresses #214 somewhat.
From my understanding the current flow in the controller is:
These changes do not allow for:
This way, the controller does not need to be modified to perform multiple
step 1s followed bystep 2sThe main change is that
MigrateSlotRequestaccepts a SlotRange instead of a int.slot = -1used to mean no migration is happening. Nowslot = nilis the equivalent.