-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Connect resharding finish operation #4503
Conversation
// TODO(resharding): don't commit resharding here, commit through consensus in driver | ||
self.commit_write_hashring(resharding_key.clone())?; |
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.
This is here until we propose hash ring changes through consensus in the driver. That'll be in a separate PR.
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.
I don't like it, but OK. 😂
Though, I'd structure finish_resharding
like this:
pub fn finish_resharding(&mut self, resharding_key: ReshardKey) -> CollectionResult<()> {
// TODO(resharding): don't commit resharding here, commit through consensus in driver
self.commit_write_hashring(resharding_key.clone())?;
self.check_resharding(&resharding_key, |_| {
// TODO(resharding): Check resharding is in the correct state to finish resharding!
Ok(())
})?;
self.resharding_state.write(|state| {
state.take();
})?;
Ok(())
}
I.e., put commit_write_hashring
call before check_resharding
.
Then it panics due to surrounding checks. We can also implement the consensus calls for setting resharding states first, because then we don't have to do this. |
3ea6d23
to
7c525fe
Compare
I moved this on top of #4507 so that we don't need the temporary 'commit write' hack anymore. This now just connects the finish resharding consensus message. |
...and tweak `commit_read`/`commit_write`/`finish` ops to work properly
* End resharding and clean up * Track resharding stage in `ReshardState`... ...and tweak `commit_read`/`commit_write`/`finish` ops to work properly * Refactor hashring check for migrating and deleting migrated points * When propagating deletes, only wait for the last batch --------- Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com>
Tracked in: #4213
Depends on: #4507
When the resharding driver is done, properly end resharding and clean things up.
This is one of the necessary changes we need to complete a resharding operation front to end. I'd like this so we can start testing with it soon.
Tasks
dev
All Submissions:
dev
branch. Did you create your branch fromdev
?New Feature Submissions:
cargo +nightly fmt --all
command prior to submission?cargo clippy --all --all-features
command?