From 281af0de86b17244a2c64af80db0bf84de6d9819 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 6 Apr 2020 16:46:52 -0400 Subject: [PATCH] rbd-mirror: prune unnecessary non-primary mirror snapshots Once a non-primary snapshot is no longer required for syncing, delete it from the image. Fixes: https://tracker.ceph.com/issues/44105 Signed-off-by: Jason Dillaman --- .../snapshot/test_mock_Replayer.cc | 32 +++++++- .../image_replayer/snapshot/Replayer.cc | 81 +++++++++++++++++++ .../image_replayer/snapshot/Replayer.h | 6 ++ 3 files changed, 118 insertions(+), 1 deletion(-) 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 4995347a6f9b7..17407e0e46653 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 @@ -18,6 +18,7 @@ #include "tools/rbd_mirror/image_replayer/snapshot/StateBuilder.h" #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librbd/mock/MockImageCtx.h" +#include "test/librbd/mock/MockOperations.h" #include "test/rbd_mirror/mock/MockContextWQ.h" #include "test/rbd_mirror/mock/MockSafeTimer.h" @@ -456,6 +457,22 @@ public: })); } + void expect_prune_non_primary_snapshot(librbd::MockTestImageCtx& mock_image_ctx, + uint64_t snap_id, int r) { + EXPECT_CALL(mock_image_ctx, get_snap_info(snap_id)) + .WillOnce(Invoke([&mock_image_ctx](uint64_t snap_id) -> librbd::SnapInfo* { + auto it = mock_image_ctx.snap_info.find(snap_id); + if (it == mock_image_ctx.snap_info.end()) { + return nullptr; + } + return &it->second; + })); + EXPECT_CALL(*mock_image_ctx.operations, snap_remove(_, _, _)) + .WillOnce(WithArg<2>(Invoke([this, r](Context* ctx) { + m_threads->work_queue->queue(ctx, r); + }))); + } + void expect_snapshot_copy(MockSnapshotCopyRequest& mock_snapshot_copy_request, uint64_t src_snap_id_start, uint64_t src_snap_id_end, @@ -786,7 +803,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { 0); expect_notify_sync_complete(mock_instance_watcher, mock_local_image_ctx.id); - // idle + // prune non-primary snap1 expect_load_image_meta(mock_image_meta, false, 0); expect_is_refresh_required(mock_local_image_ctx, true); expect_refresh( @@ -803,6 +820,19 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, SyncSnapshot) { 0, {}, 0, 0, {}}}, }, 0); expect_is_refresh_required(mock_remote_image_ctx, false); + expect_prune_non_primary_snapshot(mock_local_image_ctx, 11, 0); + + // idle + expect_load_image_meta(mock_image_meta, false, 0); + expect_is_refresh_required(mock_local_image_ctx, true); + expect_refresh( + mock_local_image_ctx, { + {14U, librbd::SnapInfo{"snap4", cls::rbd::MirrorSnapshotNamespace{ + cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "remote mirror uuid", + 4, true, 0, {}}, + 0, {}, 0, 0, {}}}, + }, 0); + expect_is_refresh_required(mock_remote_image_ctx, false); // fire init C_SaferCond init_ctx; diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc index 1979f5c5a1bce..a28cb82e3bfa9 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc @@ -11,6 +11,7 @@ #include "json_spirit/json_spirit.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" +#include "librbd/Operations.h" #include "librbd/Utils.h" #include "librbd/deep_copy/Handler.h" #include "librbd/deep_copy/ImageCopyRequest.h" @@ -29,6 +30,7 @@ #include "tools/rbd_mirror/image_replayer/snapshot/ApplyImageStateRequest.h" #include "tools/rbd_mirror/image_replayer/snapshot/StateBuilder.h" #include "tools/rbd_mirror/image_replayer/snapshot/Utils.h" +#include #define dout_context g_ceph_context #define dout_subsys ceph_subsys_rbd_mirror @@ -416,6 +418,8 @@ void Replayer::scan_local_mirror_snapshots( m_remote_snap_id_end = CEPH_NOSNAP; m_remote_mirror_snap_ns = {}; + std::set prune_snap_ids; + auto local_image_ctx = m_state_builder->local_image_ctx; std::shared_lock image_locker{local_image_ctx->image_lock}; for (auto snap_info_it = local_image_ctx->snap_info.begin(); @@ -437,10 +441,24 @@ void Replayer::scan_local_mirror_snapshots( // if remote has new snapshots, we would sync from here m_local_snap_id_start = local_snap_id; m_local_snap_id_end = CEPH_NOSNAP; + + if (mirror_ns->mirror_peer_uuids.empty()) { + // no other peer will attempt to sync to this snapshot so store as + // a candidate for removal + prune_snap_ids.insert(local_snap_id); + } } else { // start snap will be last complete mirror snapshot or initial // image revision m_local_snap_id_end = local_snap_id; + + if (mirror_ns->last_copied_object_number == 0) { + // snapshot might be missing image state, object-map, etc, so just + // delete and re-create it if we haven't started copying data + // objects + prune_snap_ids.insert(local_snap_id); + break; + } } } else if (mirror_ns->is_primary()) { if (mirror_ns->complete) { @@ -461,6 +479,19 @@ void Replayer::scan_local_mirror_snapshots( } image_locker.unlock(); + if (m_local_snap_id_start > 0 && m_local_snap_id_end == CEPH_NOSNAP) { + // remove candidate that is required for delta snapshot sync + prune_snap_ids.erase(m_local_snap_id_start); + } + if (!prune_snap_ids.empty()) { + locker->unlock(); + + auto prune_snap_id = *prune_snap_ids.begin(); + dout(5) << "pruning unused non-primary snapshot " << prune_snap_id << dendl; + prune_non_primary_snapshot(prune_snap_id); + return; + } + if (m_local_snap_id_start > 0 || m_local_snap_id_end != CEPH_NOSNAP) { if (m_local_mirror_snap_ns.is_non_primary() && m_local_mirror_snap_ns.primary_mirror_uuid != @@ -656,6 +687,56 @@ void Replayer::scan_remote_mirror_snapshots( notify_status_updated(); } +template +void Replayer::prune_non_primary_snapshot(uint64_t snap_id) { + dout(10) << "snap_id=" << snap_id << dendl; + + auto local_image_ctx = m_state_builder->local_image_ctx; + bool snap_valid = false; + cls::rbd::SnapshotNamespace snap_namespace; + std::string snap_name; + + { + std::shared_lock image_locker{local_image_ctx->image_lock}; + auto snap_info = local_image_ctx->get_snap_info(snap_id); + if (snap_info != nullptr) { + snap_valid = true; + snap_namespace = snap_info->snap_namespace; + snap_name = snap_info->name; + + ceph_assert(boost::get( + &snap_namespace) != nullptr); + } + } + + if (!snap_valid) { + load_local_image_meta(); + return; + } + + auto ctx = create_context_callback< + Replayer, &Replayer::handle_prune_non_primary_snapshot>(this); + local_image_ctx->operations->snap_remove(snap_namespace, snap_name, ctx); +} + +template +void Replayer::handle_prune_non_primary_snapshot(int r) { + dout(10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + derr << "failed to prune non-primary snapshot: " << cpp_strerror(r) + << dendl; + handle_replay_complete(r, "failed to prune non-primary snapshot"); + return; + } + + if (is_replay_interrupted()) { + return; + } + + load_local_image_meta(); +} + template void Replayer::copy_snapshots() { dout(10) << "remote_snap_id_start=" << m_remote_snap_id_start << ", " diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h index 4a96291d614e3..1b50a3d1489ff 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h @@ -115,6 +115,9 @@ private: * v (skip if not needed) | * REFRESH_REMOTE_IMAGE | * | | + * | (unused non-primary snapshot) | + * |\--------------> PRUNE_NON_PRIMARY_SNAPSHOT---/| + * | | * | (interrupted sync) | * |\--------------> GET_LOCAL_IMAGE_STATE ------\ | * | | | @@ -252,6 +255,9 @@ private: void scan_local_mirror_snapshots(std::unique_lock* locker); void scan_remote_mirror_snapshots(std::unique_lock* locker); + void prune_non_primary_snapshot(uint64_t snap_id); + void handle_prune_non_primary_snapshot(int r); + void copy_snapshots(); void handle_copy_snapshots(int r); -- 2.39.5