-
Notifications
You must be signed in to change notification settings - Fork 658
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
[resharding] Introduce ReshardingManager #12149
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12149 +/- ##
==========================================
- Coverage 71.59% 71.58% -0.01%
==========================================
Files 823 824 +1
Lines 165137 165173 +36
Branches 165137 165173 +36
==========================================
+ Hits 118223 118236 +13
- Misses 41785 41806 +21
- Partials 5129 5131 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Side note: I have something in an upcoming PR which might be more suited for resharding manager. It's about categorizing resharding events. |
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.
Looks great!
/// Configuration for resharding. | ||
pub resharding_config: MutableConfigValue<ReshardingConfig>, |
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.
Not for this PR; The old config won't make much sense for Instant Resharding. We can reuse the same config for it or introduce a separate config. I think we need to keep the old fields for now as we still use it for the archival nodes recovery.
/// A handle that allows the main process to interrupt resharding if needed. | ||
/// This typically happens when the main process is interrupted. | ||
pub resharding_handle: ReshardingHandle, |
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.
FYI @Trisfald as we discussed interrupting resharding. This handle is how we did it in V2, it's very simple and it's already integrated so we can reuse that.
The resharding manager should hopefully be a one stop shop for all things resharding.
For now I'm keeping it as a member of Chain instead of Client but we can change that later.
The resharding manager in the future should be able to do the following work
There are some complications related to accessing store components that I plan to simplify in the future