From: Jason Dillaman Date: Mon, 7 May 2018 21:43:13 +0000 (-0700) Subject: rbd-mirror: prevent blacklisting of local instance after failover/back X-Git-Tag: v13.1.1~1^2~21 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4ec33eacc49085384a01058a692d1d8c740d6030;p=ceph.git rbd-mirror: prevent blacklisting of local instance after failover/back If the leader role is manually released, upon failback the instance will have removed its local instance object, preventing RPC messaging. Signed-off-by: Jason Dillaman (cherry picked from commit ce974305893c4262b62547ce184d514578358218) --- diff --git a/src/test/rbd_mirror/test_Instances.cc b/src/test/rbd_mirror/test_Instances.cc index f6a80e572b7..b9085352ff2 100644 --- a/src/test/rbd_mirror/test_Instances.cc +++ b/src/test/rbd_mirror/test_Instances.cc @@ -58,15 +58,18 @@ public: TestFixture::SetUp(); m_local_io_ctx.remove(RBD_MIRROR_LEADER); EXPECT_EQ(0, m_local_io_ctx.create(RBD_MIRROR_LEADER, true)); + + m_instance_id = stringify(m_local_io_ctx.get_instance_id()); } Listener m_listener; + std::string m_instance_id; }; TEST_F(TestInstances, InitShutdown) { m_listener.add.count = 1; - Instances<> instances(m_threads, m_local_io_ctx, m_listener); + Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener); std::string instance_id = "instance_id"; ASSERT_EQ(0, librbd::cls_client::mirror_instances_add(&m_local_io_ctx, @@ -89,7 +92,7 @@ TEST_F(TestInstances, InitShutdown) TEST_F(TestInstances, InitEnoent) { - Instances<> instances(m_threads, m_local_io_ctx, m_listener); + Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener); m_local_io_ctx.remove(RBD_MIRROR_LEADER); @@ -113,7 +116,7 @@ TEST_F(TestInstances, NotifyRemove) m_listener.add.count = 2; m_listener.remove.count = 1; - Instances<> instances(m_threads, m_local_io_ctx, m_listener); + Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener); std::string instance_id1 = "instance_id1"; std::string instance_id2 = "instance_id2"; diff --git a/src/test/rbd_mirror/test_mock_LeaderWatcher.cc b/src/test/rbd_mirror/test_mock_LeaderWatcher.cc index f4eae19b9e3..85c9c3c785c 100644 --- a/src/test/rbd_mirror/test_mock_LeaderWatcher.cc +++ b/src/test/rbd_mirror/test_mock_LeaderWatcher.cc @@ -212,7 +212,9 @@ struct Instances { static Instances* s_instance; static Instances *create(Threads *threads, - librados::IoCtx &ioctx, instances::Listener&) { + librados::IoCtx &ioctx, + const std::string& instance_id, + instances::Listener&) { assert(s_instance != nullptr); return s_instance; } @@ -453,6 +455,10 @@ public: EXPECT_CALL(mock_instances, unblock_listener()); } + void expect_instances_acked(MockInstances& mock_instances) { + EXPECT_CALL(mock_instances, acked(_)); + } + MockThreads *m_mock_threads; }; @@ -479,6 +485,7 @@ TEST_F(TestMockLeaderWatcher, InitShutdown) { expect_acquire_notify(mock_managed_lock, listener, 0); expect_unblock_listener(mock_instances); expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish); + expect_instances_acked(mock_instances); ASSERT_EQ(0, leader_watcher.init()); ASSERT_EQ(0, on_heartbeat_finish.wait()); @@ -517,6 +524,7 @@ TEST_F(TestMockLeaderWatcher, InitReleaseShutdown) { expect_acquire_notify(mock_managed_lock, listener, 0); expect_unblock_listener(mock_instances); expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish); + expect_instances_acked(mock_instances); ASSERT_EQ(0, leader_watcher.init()); ASSERT_EQ(0, on_heartbeat_finish.wait()); @@ -592,6 +600,7 @@ TEST_F(TestMockLeaderWatcher, AcquireError) { expect_acquire_notify(mock_managed_lock, listener, 0); expect_unblock_listener(mock_instances); expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish); + expect_instances_acked(mock_instances); ASSERT_EQ(0, leader_watcher.init()); ASSERT_EQ(0, on_heartbeat_finish.wait()); @@ -647,6 +656,7 @@ TEST_F(TestMockLeaderWatcher, Break) { expect_acquire_notify(mock_managed_lock, listener, 0); expect_unblock_listener(mock_instances); expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish); + expect_instances_acked(mock_instances); ASSERT_EQ(0, leader_watcher.init()); ASSERT_EQ(0, on_heartbeat_finish.wait()); diff --git a/src/tools/rbd_mirror/Instances.cc b/src/tools/rbd_mirror/Instances.cc index 7f4033570b6..17d81bfdf6e 100644 --- a/src/tools/rbd_mirror/Instances.cc +++ b/src/tools/rbd_mirror/Instances.cc @@ -26,9 +26,10 @@ using librbd::util::create_rados_callback; template Instances::Instances(Threads *threads, librados::IoCtx &ioctx, + const std::string& instance_id, instances::Listener& listener) : - m_threads(threads), m_ioctx(ioctx), m_listener(listener), - m_cct(reinterpret_cast(ioctx.cct())), + m_threads(threads), m_ioctx(ioctx), m_instance_id(instance_id), + m_listener(listener), m_cct(reinterpret_cast(ioctx.cct())), m_lock("rbd::mirror::Instances " + ioctx.get_pool_name()) { } @@ -243,6 +244,9 @@ void Instances::remove_instances(const utime_t& time) { InstanceIds instance_ids; for (auto& instance_pair : m_instances) { + if (instance_pair.first == m_instance_id) { + continue; + } auto& instance = instance_pair.second; if (instance.state != INSTANCE_STATE_REMOVING && instance.acked_time <= time) { @@ -317,6 +321,9 @@ void Instances::schedule_remove_task(const utime_t& time) { bool schedule = false; utime_t oldest_time = time; for (auto& instance : m_instances) { + if (instance.first == m_instance_id) { + continue; + } if (instance.second.state == INSTANCE_STATE_REMOVING) { // removal is already in-flight continue; diff --git a/src/tools/rbd_mirror/Instances.h b/src/tools/rbd_mirror/Instances.h index 3a4f5f3e844..f36cc2df9d7 100644 --- a/src/tools/rbd_mirror/Instances.h +++ b/src/tools/rbd_mirror/Instances.h @@ -28,15 +28,16 @@ public: static Instances *create(Threads *threads, librados::IoCtx &ioctx, + const std::string& instance_id, instances::Listener& listener) { - return new Instances(threads, ioctx, listener); + return new Instances(threads, ioctx, instance_id, listener); } void destroy() { delete this; } Instances(Threads *threads, librados::IoCtx &ioctx, - instances::Listener& listener); + const std::string& instance_id, instances::Listener& listener); virtual ~Instances(); void init(Context *on_finish); @@ -129,6 +130,7 @@ private: Threads *m_threads; librados::IoCtx &m_ioctx; + std::string m_instance_id; instances::Listener& m_listener; CephContext *m_cct; diff --git a/src/tools/rbd_mirror/LeaderWatcher.cc b/src/tools/rbd_mirror/LeaderWatcher.cc index f9edf021b77..8f0c2c9654c 100644 --- a/src/tools/rbd_mirror/LeaderWatcher.cc +++ b/src/tools/rbd_mirror/LeaderWatcher.cc @@ -32,6 +32,7 @@ LeaderWatcher::LeaderWatcher(Threads *threads, librados::IoCtx &io_ctx, m_threads(threads), m_listener(listener), m_instances_listener(this), m_lock("rbd::mirror::LeaderWatcher " + io_ctx.get_pool_name()), m_notifier_id(librados::Rados(io_ctx).get_instance_id()), + m_instance_id(stringify(m_notifier_id)), m_leader_lock(new LeaderLock(m_ioctx, m_work_queue, m_oid, this, true, m_cct->_conf->get_val( "rbd_blacklist_expire_seconds"))) { @@ -48,7 +49,7 @@ LeaderWatcher::~LeaderWatcher() { template std::string LeaderWatcher::get_instance_id() { - return stringify(m_notifier_id); + return m_instance_id; } template @@ -284,7 +285,7 @@ bool LeaderWatcher::get_leader_instance_id(std::string *instance_id) const { Mutex::Locker locker(m_lock); if (is_leader(m_lock) || is_releasing_leader(m_lock)) { - *instance_id = stringify(m_notifier_id); + *instance_id = m_instance_id; return true; } @@ -765,7 +766,8 @@ void LeaderWatcher::init_instances() { assert(m_lock.is_locked()); assert(m_instances == nullptr); - m_instances = Instances::create(m_threads, m_ioctx, m_instances_listener); + m_instances = Instances::create(m_threads, m_ioctx, m_instance_id, + m_instances_listener); Context *ctx = create_context_callback< LeaderWatcher, &LeaderWatcher::handle_init_instances>(this); @@ -992,10 +994,6 @@ void LeaderWatcher::handle_notify_heartbeat(int r) { std::vector instance_ids; for (auto &it: m_heartbeat_response.acks) { uint64_t notifier_id = it.first.gid; - if (notifier_id == m_notifier_id) { - continue; - } - instance_ids.push_back(stringify(notifier_id)); } if (!instance_ids.empty()) { diff --git a/src/tools/rbd_mirror/LeaderWatcher.h b/src/tools/rbd_mirror/LeaderWatcher.h index b21a7816977..14571357c09 100644 --- a/src/tools/rbd_mirror/LeaderWatcher.h +++ b/src/tools/rbd_mirror/LeaderWatcher.h @@ -210,6 +210,7 @@ private: InstancesListener m_instances_listener; mutable Mutex m_lock; uint64_t m_notifier_id; + std::string m_instance_id; LeaderLock *m_leader_lock; Context *m_on_finish = nullptr; Context *m_on_shut_down_finish = nullptr;