From: Miki Patel Date: Tue, 21 Apr 2026 11:53:29 +0000 (+0530) Subject: rbd-mirror: Remove old non-primary demoted image snapshots on the local cluster X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5a77c4fd359e485501256fa3b21272fb06e8f0af;p=ceph.git 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 --- diff --git a/qa/workunits/rbd/rbd_mirror.sh b/qa/workunits/rbd/rbd_mirror.sh index 99166f169ba..b19005c9d36 100755 --- 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 a2dabf2e0ae..07f7a4b168c 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 @@ -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 b85edfc105d..10167185d4a 100644 --- 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::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);