diff --git a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc index 17407e0e466..02cbc768460 100644 --- a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc +++ b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc @@ -819,7 +819,20 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { 4, true, 0, {}}, 0, {}, 0, 0, {}}}, }, 0); - expect_is_refresh_required(mock_remote_image_ctx, false); + expect_is_refresh_required(mock_remote_image_ctx, true); + expect_refresh( + mock_remote_image_ctx, { + {2U, librbd::SnapInfo{"snap2", cls::rbd::UserSnapshotNamespace{}, + 0, {}, 0, 0, {}}}, + {3U, librbd::SnapInfo{"snap3", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {""}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {4U, librbd::SnapInfo{"snap4", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}} + }, 0); expect_prune_non_primary_snapshot(mock_local_image_ctx, 11, 0); // idle @@ -2079,7 +2092,19 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, RemoteFailover) { {{1, 11}, {2, 12}, {3, CEPH_NOSNAP}}}, 0, {}, 0, 0, {}}}, }, 0); - expect_is_refresh_required(mock_remote_image_ctx, false); + expect_is_refresh_required(mock_remote_image_ctx, true); + expect_refresh( + mock_remote_image_ctx, { + {1U, librbd::SnapInfo{"snap1", cls::rbd::UserSnapshotNamespace{}, + 0, {}, 0, 0, {}}}, + {2U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY_DEMOTED, + {"remote mirror peer uuid"}, "local mirror uuid", 12U, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {3U, librbd::SnapInfo{"snap3", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {}, "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}} + }, 0); // wake-up replayer update_watch_ctx->handle_notify(); @@ -2091,6 +2116,95 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, RemoteFailover) { mock_remote_image_ctx)); } +TEST_F(TestMockImageReplayerSnapshotReplayer, UnlinkRemoteSnapshot) { + librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx}; + librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx}; + + // it should attempt to unlink from remote snap1 since we don't need it + // anymore + mock_local_image_ctx.snap_info = { + {14U, librbd::SnapInfo{"snap4", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 4, true, 0, {}}, + 0, {}, 0, 0, {}}}}; + mock_remote_image_ctx.snap_info = { + {1U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {4U, librbd::SnapInfo{"snap4", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}}}; + + MockThreads mock_threads(m_threads); + expect_work_queue_repeatedly(mock_threads); + + MockReplayerListener mock_replayer_listener; + expect_notification(mock_threads, mock_replayer_listener); + + InSequence seq; + + MockInstanceWatcher mock_instance_watcher; + MockImageMeta mock_image_meta; + MockStateBuilder mock_state_builder(mock_local_image_ctx, + mock_remote_image_ctx, + mock_image_meta); + MockReplayer mock_replayer{&mock_threads, &mock_instance_watcher, + "local mirror uuid", &m_pool_meta_cache, + &mock_state_builder, &mock_replayer_listener}; + m_pool_meta_cache.set_remote_pool_meta( + m_remote_io_ctx.get_id(), + {"remote mirror uuid", "remote mirror peer uuid"}); + + librbd::UpdateWatchCtx* update_watch_ctx = nullptr; + + // init + expect_register_update_watcher(mock_local_image_ctx, &update_watch_ctx, 123, + 0); + expect_register_update_watcher(mock_remote_image_ctx, &update_watch_ctx, 234, + 0); + + // unlink snap1 + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, false); + expect_is_refresh_required(mock_remote_image_ctx, false); + MockUnlinkPeerRequest mock_unlink_peer_request; + expect_unlink_peer(mock_unlink_peer_request, 1, "remote mirror peer uuid", + 0); + + // idle + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, false); + expect_is_refresh_required(mock_remote_image_ctx, true); + expect_refresh( + mock_remote_image_ctx, { + {2U, librbd::SnapInfo{"snap2", cls::rbd::UserSnapshotNamespace{}, + 0, {}, 0, 0, {}}}, + {3U, librbd::SnapInfo{"snap3", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {""}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}}, + {4U, librbd::SnapInfo{"snap4", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {"remote mirror peer uuid"}, + "", 0U, true, 0, {}}, + 0, {}, 0, 0, {}}} + }, 0); + + // fire init + C_SaferCond init_ctx; + mock_replayer.init(&init_ctx); + ASSERT_EQ(0, init_ctx.wait()); + + // wait for sync to complete + ASSERT_EQ(0, wait_for_notification(2)); + + // shut down + ASSERT_EQ(0, shut_down_entry_replayer(mock_replayer, mock_threads, + mock_local_image_ctx, + mock_remote_image_ctx)); +} + } // namespace snapshot } // namespace image_replayer } // namespace mirror diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc index a28cb82e3bf..7980f525491 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc @@ -532,6 +532,7 @@ void Replayer::scan_remote_mirror_snapshots( m_pending_snapshots = 0; + std::set unlink_snap_ids; bool split_brain = false; bool remote_demoted = false; auto remote_image_ctx = m_state_builder->remote_image_ctx; @@ -547,13 +548,17 @@ void Replayer::scan_remote_mirror_snapshots( dout(15) << "remote mirror snapshot: id=" << snap_info_it->first << ", " << "mirror_ns=" << *mirror_ns << dendl; + remote_demoted = mirror_ns->is_demoted(); if (!mirror_ns->is_primary() && !mirror_ns->is_non_primary()) { derr << "unknown remote mirror snapshot state" << dendl; handle_replay_complete(locker, -EINVAL, "invalid remote mirror snapshot state"); return; - } else { - remote_demoted = mirror_ns->is_demoted(); + } else if (mirror_ns->mirror_peer_uuids.count(m_remote_mirror_peer_uuid) == + 0) { + dout(15) << "skipping remote snapshot due to missing mirror peer" + << dendl; + continue; } auto remote_snap_id = snap_info_it->first; @@ -564,10 +569,12 @@ void Replayer::scan_remote_mirror_snapshots( ceph_assert(m_local_mirror_snap_ns.primary_mirror_uuid == m_state_builder->remote_mirror_uuid); + unlink_snap_ids.insert(remote_snap_id); if (m_local_mirror_snap_ns.complete && m_local_mirror_snap_ns.primary_snap_id >= remote_snap_id) { // skip past completed remote snapshot m_remote_snap_id_start = remote_snap_id; + m_remote_mirror_snap_ns = *mirror_ns; dout(15) << "skipping synced remote snapshot " << remote_snap_id << dendl; continue; @@ -577,6 +584,7 @@ void Replayer::scan_remote_mirror_snapshots( dout(15) << "skipping synced remote snapshot " << remote_snap_id << " while search for in-progress sync" << dendl; m_remote_snap_id_start = remote_snap_id; + m_remote_mirror_snap_ns = *mirror_ns; continue; } } else if (m_local_mirror_snap_ns.state == @@ -604,17 +612,9 @@ void Replayer::scan_remote_mirror_snapshots( // should not have been able to reach this ceph_assert(false); } - } - - // find first snapshot where were are listed as a peer - if (!mirror_ns->is_primary()) { + } else if (!mirror_ns->is_primary()) { dout(15) << "skipping non-primary remote snapshot" << dendl; continue; - } else if (mirror_ns->mirror_peer_uuids.count(m_remote_mirror_peer_uuid) == - 0) { - dout(15) << "skipping remote snapshot due to missing mirror peer" - << dendl; - continue; } // found candidate snapshot to sync @@ -623,11 +623,25 @@ void Replayer::scan_remote_mirror_snapshots( continue; } + // first primary snapshot where were are listed as a peer m_remote_snap_id_end = remote_snap_id; m_remote_mirror_snap_ns = *mirror_ns; } image_locker.unlock(); + unlink_snap_ids.erase(m_remote_snap_id_start); + unlink_snap_ids.erase(m_remote_snap_id_end); + if (!unlink_snap_ids.empty()) { + locker->unlock(); + + // retry the unlinking process for a remote snapshot that we do not + // need anymore + auto remote_snap_id = *unlink_snap_ids.begin(); + dout(10) << "unlinking from remote snapshot " << remote_snap_id << dendl; + unlink_peer(remote_snap_id); + return; + } + if (m_remote_snap_id_end != CEPH_NOSNAP) { dout(10) << "found remote mirror snapshot: " << "remote_snap_id_start=" << m_remote_snap_id_start << ", " @@ -1143,24 +1157,24 @@ void Replayer::handle_notify_image_update(int r) { derr << "failed to notify local image update: " << cpp_strerror(r) << dendl; } - unlink_peer(); + unlink_peer(m_remote_snap_id_start); } template -void Replayer::unlink_peer() { - if (m_remote_snap_id_start == 0) { +void Replayer::unlink_peer(uint64_t remote_snap_id) { + if (remote_snap_id == 0) { finish_sync(); return; } // local snapshot fully synced -- we no longer depend on the sync // start snapshot in the remote image - dout(10) << "remote_snap_id=" << m_remote_snap_id_start << dendl; + dout(10) << "remote_snap_id=" << remote_snap_id << dendl; auto ctx = create_context_callback< Replayer, &Replayer::handle_unlink_peer>(this); auto req = librbd::mirror::snapshot::UnlinkPeerRequest::create( - m_state_builder->remote_image_ctx, m_remote_snap_id_start, + m_state_builder->remote_image_ctx, remote_snap_id, m_remote_mirror_peer_uuid, ctx); req->send(); } @@ -1187,9 +1201,11 @@ void Replayer::finish_sync() { std::unique_lock locker{m_lock}; notify_status_updated(); - m_sync_in_progress = false; - m_instance_watcher->notify_sync_complete( - m_state_builder->local_image_ctx->id); + if (m_sync_in_progress) { + m_sync_in_progress = false; + m_instance_watcher->notify_sync_complete( + m_state_builder->local_image_ctx->id); + } } if (is_replay_interrupted()) { diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h index 1b50a3d1489..dda101b0c2a 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h @@ -123,37 +123,37 @@ private: * | | | * | (new snapshot) | | * |\--------------> COPY_SNAPSHOTS | | - * | | | | - * | v | | + * | | | | + * | v | | * | GET_REMOTE_IMAGE_STATE | | - * | | | | - * | v | | + * | | | | + * | v | | * | CREATE_NON_PRIMARY_SNAPSHOT | | - * | | | | - * | |/----------------------/ | - * | | | - * | v | + * | | | | + * | |/--------------------/ | + * | | | + * | v | * | REQUEST_SYNC | - * | | | - * | v | + * | | | + * | v | * | COPY_IMAGE | - * | | | - * | v | + * | | | + * | v | * | APPLY_IMAGE_STATE | - * | | | - * | v | + * | | | + * | v | * | UPDATE_NON_PRIMARY_SNAPSHOT | - * | | | - * | v | + * | | | + * | v | * | NOTIFY_IMAGE_UPDATE | - * | | | - * | v | - * | UNLINK_PEER | - * | | | - * | v | + * | | | + * | (interrupted unlink) v | + * |\--------------> UNLINK_PEER | + * | | | + * | v | * | NOTIFY_LISTENER | - * | | | - * | \------------------------/| + * | | | + * | \----------------------/| * | | * | (remote demoted) | * \---------------> NOTIFY_LISTENER | @@ -288,7 +288,7 @@ private: void notify_image_update(); void handle_notify_image_update(int r); - void unlink_peer(); + void unlink_peer(uint64_t remote_snap_id); void handle_unlink_peer(int r); void finish_sync();