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
rbd-mirror: integrate image map policy as incremental step to active-active #21300
Conversation
src/tools/rbd_mirror/LeaderWatcher.h
Outdated
* . | ^ . .....^..................... | ||
* . v (error) | . | | ||
* .INIT_INSTANCES *> SHUT_DOWN_STATUS_WATCHER . RELEASE_LEADER_LOCK | ||
* .INIT_INSTANCES *> SHUT_DOWN_STATUS_WATCHER . .NOTIFY_LOCK_RELEASED . |
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.
@dillaman Need to remove SHUT_DOWN_STATUS_WATCHER
here too.
if (r < 0) { | ||
Mutex::Locker locker(m_lock); | ||
derr << "error initializing instances: " << cpp_strerror(r) << dendl; | ||
m_ret_val = r; |
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.
It looks like there is no need to set m_ret_val
here any more.
897179d
to
cc0acd4
Compare
src/tools/rbd_mirror/Instances.cc
Outdated
} | ||
} | ||
|
||
dout(5) << "instance_ids=" << added_instance_ids << dendl; |
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.
Probably skip the part below if added_instance_ids
happens to be empty?
src/tools/rbd_mirror/Instances.cc
Outdated
m_listener.handle_added(added_instance_ids); | ||
m_lock.Lock(); | ||
|
||
for (auto& instance_id : instance_ids) { |
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.
Shouldn't we iterate over added_instance_ids
here?
7b5986e
to
f1d2fab
Compare
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
…class Signed-off-by: Jason Dillaman <dillaman@redhat.com>
In the case of active/active, both the leader and the followers will be sending status updates for their images. Therefore, all rbd-mirror daemons should be registered. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
This throttles the on-disk updates and also will eventually help to throttle the shuffling of images between alive peer instances. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Avoid a possible race condition with instances being detected before the pool replayer leader handling is fully initialized. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
The context wraps the necessary data and provides a pass-through chain to the instance notification helpers. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
The context callbacks between the image map and its policy required too many hooks between the two classes. Additionally, the original implementation didn't support re-sending of acquire messages after initializing (to close a potential race condition). Signed-off-by: Jason Dillaman <dillaman@redhat.com>
At initial promotion to the leader position, it can be nearly always assumed that at least one instance has died and needs to have its images shuffled to another instance (i.e. the old leader). Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
f1d2fab
to
10522f4
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.
LGTM
@ceph-jenkins retest this please |
No description provided.