librbd/mirror: tweak which snapshot is unlinked when at capacity

The rbd-mirror daemon will attempt to sync from the last synced
snapshot to the next mirror snapshot. When the limit is at 3, this
currently can result in a situation where an in-use sync snapshot is
deleted. Instead of unlinking the second oldest snapshot, always
unlink the third oldest.

Fixes: https://tracker.ceph.com/issues/48553
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
index 70962cc6efbc2051c28ee60693a4a9333524f41b..ce14df952e981278db69b9b5bb24a4560f23d326 100644 (file)
--- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
+++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
@@ -192,7 +192,7 @@ void CreatePrimaryRequest<I>::unlink_peer() {
   for (auto &peer : m_mirror_peer_uuids) {
     std::shared_lock image_locker{m_image_ctx->image_lock};
     size_t count = 0;
-    uint64_t prev_snap_id = 0;
+    uint64_t unlink_snap_id = 0;
     for (auto &snap_it : m_image_ctx->snap_info) {
       auto info = boost::get<cls::rbd::MirrorSnapshotNamespace>(
         &snap_it.second.snap_namespace);
@@ -202,15 +202,16 @@ void CreatePrimaryRequest<I>::unlink_peer() {
       if (info->state != cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY) {
         // reset counters -- we count primary snapshots after the last promotion
         count = 0;
-        prev_snap_id = 0;
+        unlink_snap_id = 0;
         continue;
       }
       count++;
-      if (count == max_snapshots - 1) {
-        prev_snap_id = snap_it.first;
-      } else if (count > max_snapshots) {
+      if (count == 3) {
+        unlink_snap_id = snap_it.first;
+      }
+      if (count > max_snapshots) {
         peer_uuid = peer;
-        snap_id = prev_snap_id;
+        snap_id = unlink_snap_id;
         break;
       }
     }

diff --git a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc
index f38e58d8e87387fa71b29d77013bd0e1e465468a..2627e33a802cee32f19b3835286fd9a7e643a7bb 100644 (file)
--- a/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc
+++ b/src/test/librbd/mirror/snapshot/test_mock_CreatePrimaryRequest.cc
@@ -313,7 +313,7 @@ TEST_F(TestMockMirrorSnapshotCreatePrimaryRequest, SuccessUnlinkPeer) {
   expect_create_snapshot(mock_image_ctx, 0);
   MockUnlinkPeerRequest mock_unlink_peer_request;
   auto it = mock_image_ctx.snap_info.rbegin();
-  auto snap_id = (++it)->first;
+  auto snap_id = it->first;
   expect_unlink_peer(mock_image_ctx, mock_unlink_peer_request, snap_id, "uuid",
                      0);
   C_SaferCond ctx;

diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc
index 75d18208cc702b95a03678dbdc5e2f28733fa525..daab119fe3a4980a0a7c1a0c7ef7ac157659f462 100644 (file)
--- a/src/test/librbd/test_mirroring.cc
+++ b/src/test/librbd/test_mirroring.cc
@@ -1153,7 +1153,7 @@ TEST_F(TestMirroring, Snapshot)
   ASSERT_EQ(0, image.snap_list(snaps1));
   ASSERT_EQ(3U, snaps1.size());
   ASSERT_EQ(snaps1[0].id, snaps[0].id);
-  ASSERT_EQ(snaps1[1].id, snaps[2].id);
+  ASSERT_EQ(snaps1[1].id, snaps[1].id);
   ASSERT_EQ(snaps1[2].id, snap_id);
 
   librbd::snap_namespace_type_t snap_ns_type;