-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Abort resharding after marking as dead #6394
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
Abort resharding after marking as dead #6394
Conversation
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.
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
lib/collection/src/tests/points_dedup.rs:137
- [nitpick] Consider introducing a named constant or inline documentation for the literal 'false' parameter to clarify its purpose in the call to update_local.
.update_local(op, true, HwMeasurementAcc::new(), false)
lib/collection/src/shards/shard_holder/resharding.rs:333
- [nitpick] The literal 'true' passed into delete_local_points may benefit from a named constant or comment clarifying its semantic meaning, which can improve readability and maintainability.
.delete_local_points(filter, HwMeasurementAcc::disposable(), true) // Internal operation, no performance tracking needed
lib/collection/src/collection/mod.rs:464
- [nitpick] The repeated use of literal 'false' for aborting resharding might be made clearer by extracting it into a well-named constant to better convey its purpose in the control flow.
abort_resharding_result = self.abort_resharding(state.key(), false).await;
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.
Small comment regarding when to trigger abort_resharding
, LGTM overall
f3e0c73
to
eb78314
Compare
eb78314
to
faf71f8
Compare
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.
Code LGTM. I'm testing it locally rn to find any potential problems.
Thanks! Also doing extensive testing now. |
* Abort resharding if any Resharding replica is to be marked dead * Abort resharding before marking shard as Dead * add comments * Abort resharding after marking as dead (#6394) * Force delete points when aborting resharding, even if replica is dead * Abort resharding after marking replica as dead * Only abort resharding if we mark related replica as dead * Just rely on shard replica state * Propagate shard transfer error right away --------- Co-authored-by: Tim Visée <tim+github@visee.me>
…nt#6364) * Abort resharding if any Resharding replica is to be marked dead * Abort resharding before marking shard as Dead * add comments * Abort resharding after marking as dead (qdrant#6394) * Force delete points when aborting resharding, even if replica is dead * Abort resharding after marking replica as dead * Only abort resharding if we mark related replica as dead * Just rely on shard replica state * Propagate shard transfer error right away --------- Co-authored-by: Tim Visée <tim+github@visee.me>
Fixes on top of #6364
The main things this solves is:
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?