feat(webui): implemented cluster slot migration #341
Conversation
|
Looks great! https://github.com/apache/kvrocks-controller/blob/unstable/server/api/cluster.go#L38 Seems currently we support to migrate a "slot range" than a single slot, maybe we can support that in the UI? |
sure done👍🏻 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements cluster slot migration functionality in the web UI by adding a new migration dialog component and integrating it with the existing cluster management page. The implementation allows users to migrate slots between shards through a dedicated dialog interface.
Key changes:
- Added a new
MigrationDialogcomponent with comprehensive UI for slot migration configuration - Updated the cluster page to handle string-based slot values instead of numeric values for migration tracking
- Modified the API call to properly handle slot ranges as strings
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| webui/src/app/ui/migrationDialog.tsx | New component implementing the slot migration dialog with validation and UI controls |
| webui/src/app/namespaces/[namespace]/clusters/[cluster]/page.tsx | Updated to integrate migration dialog and handle string-based slot values |
| webui/src/app/lib/api.ts | Modified migrateSlot function to accept slot parameter as string instead of number |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| interface Shard { | ||
| index: number; | ||
| nodes: any[]; |
There was a problem hiding this comment.
The nodes property uses any[] type which provides no type safety. Consider defining a proper interface for node objects to improve type safety and code maintainability.
| nodes: any[]; | |
| // Define the Node interface. Update properties as needed to match actual node structure. | |
| interface Node { | |
| // Example properties; replace with actual node properties as appropriate. | |
| id: string; | |
| address: string; | |
| role: string; | |
| [key: string]: any; // Remove or refine as you learn more about node structure. | |
| } | |
| interface Shard { | |
| index: number; | |
| nodes: Node[]; |
| resetForm(); | ||
| } | ||
| } catch (err) { | ||
| setError("An unexpected error occurred during migration"); |
There was a problem hiding this comment.
The generic error message doesn't provide useful information to users. Consider logging the actual error details and providing a more specific error message based on the error type.
| setError("An unexpected error occurred during migration"); | |
| // Log the actual error for debugging | |
| console.error("Migration error:", err); | |
| // Show a more specific error message if available | |
| if (err && typeof err === "object" && "message" in err && typeof (err as any).message === "string") { | |
| setError((err as any).message); | |
| } else { | |
| setError("An unexpected error occurred during migration"); | |
| } |
| fetchData(); | ||
| }, [namespace, cluster, router]); | ||
|
|
||
| const refreshShardData = async () => { |
There was a problem hiding this comment.
The refreshShardData function duplicates the shard data processing logic from the main fetchData function. Consider extracting the common processing logic into a shared helper function to reduce code duplication.
| }; | ||
| } | ||
|
|
||
| if (start < 0 || end > 16383 || start > 16383 || end < 0) { |
There was a problem hiding this comment.
The condition start > 16383 || end < 0 is redundant as it's already covered by start < 0 || end > 16383. This can be simplified to improve readability.
| if (start < 0 || end > 16383 || start > 16383 || end < 0) { | |
| if (start < 0 || end > 16383) { |
There is a type error. Could you fix it? |
yes slot.toString() done in commit [TypeScript error fix] |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #341 +/- ##
============================================
+ Coverage 43.38% 47.16% +3.77%
============================================
Files 37 45 +8
Lines 2971 4425 +1454
============================================
+ Hits 1289 2087 +798
- Misses 1544 2129 +585
- Partials 138 209 +71
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:
|
3 participants
implemented cluster slot migration
Monosnap.screencast.2025-08-17.15-22-02.mp4