-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
hinted handoff: Prevent segmentation fault when initializing endpoint managers #18650
Conversation
db/hints/manager.cc
Outdated
@@ -873,11 +873,11 @@ future<> manager::perform_migration() { | |||
const auto lock = co_await seastar::get_units(_drain_lock, 1); | |||
|
|||
using lock_type = std::unique_lock<seastar::shared_mutex>; | |||
// We're taking this lock because we're about to stop endpoint managers here, wheras | |||
// We're taking this lock because we're about to stop endpoint managers here, whereas |
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.
Spelling fixes are no longer needed, they were fixed by #18567 which is already on master
If hinted handoff is still IP-based and there is a hint directory representing an IP without a corresponding mapping to a host ID in `locator::token_metadata`, an attemp to initialize its endpoint manager will result in a segmentation fault. This commit prevents that.
Before these changes, if initializing endpoint managers after the migration of hinted handoff to host ID is done throws an exception, we don't remove the flag indicating the migration is still in progress. However, the migration has, in practice, finished -- all of the hint directories have been mapped to host IDs and all of the nodes in the cluster are host-ID-based. Because of that, it makes sense to remove the flag early on.
Version 2:
|
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.
LGTM, will wait for CI before merging
🟢 CI State: SUCCESS✅ - Build Build Details:
|
Queued |
We don't attempt to create an endpoint manager for a hint directory if there is no mapping host ID–IP corresponding to the directory's name, an IP address. That prevents a segmentation fault.
Fixes #18649