rbd-mirror: Remove old non-primary demoted image snapshots on the local cluster

When an image is demoted on primary cluster and later promoted again,
a non-primary demoted image snapshot is created on the peer (secondary)
cluster. Each such promote/demote cycle on same cluster results in an
additional non-primary demoted snapshot being created on peer.
As a result, repeated promote/demote cycles on the same cluster can
lead to accumulation of stale non-primary demoted snapshots on the
secondary cluster. These snapshots are not removed immediately because
cleanup is only triggered when peer (secondary) is promoted and then
demoted.

The proposed changes ensures that such snapshots are proactively identified
and removed, preventing unbounded buildup. Also added test coverage to
verify cleanup behavior.

Fixes: https://tracker.ceph.com/issues/76155
Signed-off-by: Miki Patel <miki.patel132@gmail.com>

diff --git a/qa/workunits/rbd/rbd_mirror.sh b/qa/workunits/rbd/rbd_mirror.sh
index 99166f169ba585d0ad29f3c07b914e6f723f4886..b19005c9d360d1c255bdbaf70d31a87936488711 100755 (executable)
--- a/qa/workunits/rbd/rbd_mirror.sh
+++ b/qa/workunits/rbd/rbd_mirror.sh
@@ -355,6 +355,35 @@ if [ "${RBD_MIRROR_MODE}" = "snapshot" ]; then
   test "$(count_mirror_snaps ${CLUSTER2} ${POOL} ${image})" -le 3
 fi
 
+if [ "${RBD_MIRROR_MODE}" = "snapshot" ]; then
+  testlog "TEST: demote and promote on same cluster in loop"
+  for i in `seq 1 20`; do
+    last_demote_snap_id=''
+    demote_image ${CLUSTER2} ${POOL} ${image}
+    wait_for_image_replay_stopped ${CLUSTER1} ${POOL} ${image}
+    wait_for_status_in_pool_dir ${CLUSTER1} ${POOL} ${image} 'up+unknown'
+    wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+unknown'
+    get_newest_complete_mirror_snapshot_id ${CLUSTER2} ${POOL} ${image} last_demote_snap_id
+    wait_for_non_primary_snap_present ${CLUSTER1} ${POOL} ${image} ${last_demote_snap_id}
+
+    promote_image ${CLUSTER2} ${POOL} ${image}
+    wait_for_image_replay_started ${CLUSTER1} ${POOL} ${image}
+    wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+stopped'
+    wait_for_status_in_pool_dir ${CLUSTER1} ${POOL} ${image} 'up+replaying'
+    # check non-primary demote snapshot is removed
+    wait_for_non_primary_snap_not_present ${CLUSTER1} ${POOL} ${image} ${last_demote_snap_id}
+  done
+  # expected snapshots on CLUSTER1: non-primary
+  SNAPS=$(get_snaps_json ${CLUSTER1} ${POOL} ${image})
+  jq -e 'length == 1' <<< ${SNAPS}
+  jq -e '.[0].namespace["type"] == "mirror" and .[0].namespace["state"] == "non-primary"' <<< ${SNAPS}
+  # expected snapshots on CLUSTER2: primary demoted, primary
+  SNAPS=$(get_snaps_json ${CLUSTER2} ${POOL} ${image})
+  jq -e 'length == 2' <<< ${SNAPS}
+  jq -e '.[0].namespace["type"] == "mirror" and .[0].namespace["state"] == "demoted" and (.[0]["name"] | startswith(".mirror.primary"))' <<< ${SNAPS}
+  jq -e '.[1].namespace["type"] == "mirror" and .[1].namespace["state"] == "primary"' <<< ${SNAPS}
+fi
+
 testlog "TEST: force promote"
 force_promote_image=test_force_promote
 create_image_and_enable_mirror ${CLUSTER2} ${POOL} ${force_promote_image} ${RBD_MIRROR_MODE}

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 a2dabf2e0ae8b1dcc617975f25b79955cf2596c1..07f7a4b168c35ba3a0838b426dfc421e69214f54 100644 (file)
--- a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
+++ b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
@@ -3511,6 +3511,87 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, ApplyImageStateErrorPendingShutdow
   ASSERT_EQ(0, shutdown_ctx.wait());
 }
 
+TEST_F(TestMockImageReplayerSnapshotReplayer, PruneObsoleteNonPrimaryDemotedSnapshot) {
+  librbd::MockTestImageCtx mock_local_image_ctx{*m_local_image_ctx};
+  librbd::MockTestImageCtx mock_remote_image_ctx{*m_remote_image_ctx};
+
+  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", "local mirror peer uuid",
+                             &m_pool_meta_cache, &mock_state_builder,
+                             &mock_replayer_listener};
+  m_pool_meta_cache.set_remote_pool_meta(
+    m_remote_fsid, m_remote_io_ctx.get_id(),
+    {"remote mirror uuid", "remote mirror peer uuid"});
+
+  librbd::UpdateWatchCtx* update_watch_ctx = nullptr;
+  ASSERT_EQ(0, init_entry_replayer(mock_replayer, mock_threads,
+                                   mock_local_image_ctx,
+                                   mock_remote_image_ctx,
+                                   mock_replayer_listener,
+                                   mock_image_meta,
+                                   &update_watch_ctx));
+
+  // Remote: PRIMARY_DEMOTED snap1 + PRIMARY snap2
+  mock_remote_image_ctx.snap_info = {
+    {1U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{
+       cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY_DEMOTED,
+       {}, "", CEPH_NOSNAP, true, 0, {}},
+     0, {}, 0, 0, {}}},
+    {2U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{
+       cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY,
+       {"remote mirror peer uuid"}, "", CEPH_NOSNAP, true, 0, {}},
+     0, {}, 0, 0, {}}}};
+
+  // Local: NON_PRIMARY_DEMOTED snap1 + NON_PRIMARY snap2 (already synced)
+  mock_local_image_ctx.snap_info = {
+    {11U, librbd::SnapInfo{"snap1", cls::rbd::MirrorSnapshotNamespace{
+       cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY_DEMOTED,
+       {"local mirror peer uuid"}, "remote mirror uuid", 1, true, 0, {}},
+     0, {}, 0, 0, {}}},
+    {12U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{
+       cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY,
+       {}, "remote mirror uuid", 2, true, 0, {}},
+     0, {}, 0, 0, {}}}};
+
+  expect_load_image_meta(mock_image_meta, false, 0);
+  expect_is_refresh_required(mock_remote_image_ctx, false);
+  expect_is_refresh_required(mock_local_image_ctx, false);
+  expect_prune_mirror_snapshot(mock_local_image_ctx, 11, 0);
+
+  // idle
+  expect_load_image_meta(mock_image_meta, false, 0);
+  expect_is_refresh_required(mock_remote_image_ctx, false);
+  expect_is_refresh_required(mock_local_image_ctx, true);
+  expect_refresh(
+    mock_local_image_ctx, {
+      {12U, librbd::SnapInfo{"snap2", cls::rbd::MirrorSnapshotNamespace{
+         cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY,
+         {}, "remote mirror uuid", 2, true, 0, {}},
+       0, {}, 0, 0, {}}},
+    }, 0);
+
+  // wake-up replayer
+  update_watch_ctx->handle_notify();
+  ASSERT_EQ(0, wait_for_notification(1));
+
+  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 b85edfc105d4d4aa55fb4103e79a321a535ece62..10167185d4a66f2344419bbb76bc45719b8c7276 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
+++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
@@ -517,8 +517,11 @@ void Replayer<I>::scan_local_mirror_snapshots(
         // if remote has new snapshots, we would sync from here
         m_local_snap_id_start = local_snap_id;
         ceph_assert(m_local_snap_id_end == CEPH_NOSNAP);
-
-        if (mirror_ns->mirror_peer_uuids.empty()) {
+        const auto& peer_uuids = mirror_ns->mirror_peer_uuids;
+        if (peer_uuids.empty() ||
+            (mirror_ns->state == cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY_DEMOTED &&
+             peer_uuids.size() == 1 &&
+             peer_uuids.count(m_local_mirror_peer_uuid) == 1)) {
           // no other peer will attempt to sync to this snapshot so store as
           // a candidate for removal
           prune_snap_ids.insert(local_snap_id);