osd/PGLog.cc: Trim duplicates by number of entries

PGLog needs to trim duplicates by the number of entries rather than the versions. That way, we prevent unbounded duplicate growth.

Fixed: https://tracker.ceph.com/issues/53729
Signed-off-by: Nitzan Mordechai <nmordec@redhat.com>
(cherry picked from commit 0d253bcc09a5540fa6c724f6128fb7436ded5ec1)

diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc
index 0f184497ada981c0657b0d5c2257f4a4713b03a2..3fa711a6f1dc9f085218f42b09ed11cfac44a676 100644 (file)
--- a/src/osd/PGLog.cc
+++ b/src/osd/PGLog.cc
@@ -131,10 +131,8 @@ void PGLog::IndexedLog::trim(
     }
   }
 
-  while (!dups.empty()) {
+  while (dups.size() > cct->_conf->osd_pg_log_dups_tracked) {
     const auto& e = *dups.begin();
-    if (e.version.version >= earliest_dup_version)
-      break;
     lgeneric_subdout(cct, osd, 20) << "trim dup " << e << dendl;
     if (trimmed_dups)
       trimmed_dups->insert(e.get_key_name());

diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 06d0024743a988d8750eaf0280f7e4d912ac13ff..34ed71a72b3666ea007c13654699eb1f8ac550eb 100644 (file)
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -1395,7 +1395,7 @@ public:
     bool debug_verify_stored_missing = false
     ) {
     return read_log_and_missing(
-      store, ch, pgmeta_oid, info,
+      cct, store, ch, pgmeta_oid, info,
       log, missing, oss,
       tolerate_divergent_missing_log,
       &clear_divergent_priors,
@@ -1406,6 +1406,7 @@ public:
 
   template <typename missing_type>
   static void read_log_and_missing(
+    CephContext *cct,
     ObjectStore *store,
     ObjectStore::CollectionHandle &ch,
     ghobject_t pgmeta_oid,
@@ -1475,6 +1476,9 @@ public:
            ceph_assert(dups.back().version < dup.version);
          }
          dups.push_back(dup);
+         if (dups.size() >= cct->_conf->osd_pg_log_dups_tracked) {
+           dups.pop_front();
+         }
        } else {
          pg_log_entry_t e;
          e.decode_with_checksum(bp);

diff --git a/src/test/osd/TestPGLog.cc b/src/test/osd/TestPGLog.cc
index b08b9f0ccc288ed1d9324b5e2b88c57d072cdda0..004e3cd1c589155fd04802202538460c033afb03 100644 (file)
--- a/src/test/osd/TestPGLog.cc
+++ b/src/test/osd/TestPGLog.cc
@@ -2740,8 +2740,8 @@ TEST_F(PGLogTrimTest, TestPartialTrim)
   EXPECT_EQ(eversion_t(19, 160), write_from_dups2);
   EXPECT_EQ(2u, log.log.size());
   EXPECT_EQ(1u, trimmed2.size());
-  EXPECT_EQ(2u, log.dups.size());
-  EXPECT_EQ(1u, trimmed_dups2.size());
+  EXPECT_EQ(3u, log.dups.size());
+  EXPECT_EQ(0u, trimmed_dups2.size());
 }
 
 
@@ -3024,7 +3024,7 @@ TEST_F(PGLogTrimTest, TestTrimDups) {
 
   EXPECT_EQ(eversion_t(20, 103), write_from_dups) << log;
   EXPECT_EQ(2u, log.log.size()) << log;
-  EXPECT_EQ(3u, log.dups.size()) << log;
+  EXPECT_EQ(4u, log.dups.size()) << log;
 }
 
 // This tests trim() to make copies of
@@ -3068,7 +3068,50 @@ TEST_F(PGLogTrimTest, TestTrimDups2) {
 
   EXPECT_EQ(eversion_t(10, 100), write_from_dups) << log;
   EXPECT_EQ(4u, log.log.size()) << log;
-  EXPECT_EQ(5u, log.dups.size()) << log;
+  EXPECT_EQ(6u, log.dups.size()) << log;
+}
+
+// This tests trim() to make copies of
+// 5 log entries (107, 106, 105, 104, 103) and trim all dups
+TEST_F(PGLogTrimTest, TestTrimAllDups) {
+  SetUp(0);
+  PGLog::IndexedLog log;
+  log.head = mk_evt(21, 107);
+  log.skip_can_rollback_to_to_head();
+  log.tail = mk_evt(9, 99);
+  log.head = mk_evt(9, 99);
+
+  entity_name_t client = entity_name_t::CLIENT(777);
+
+  log.dups.push_back(pg_log_dup_t(mk_ple_mod(mk_obj(1),
+         mk_evt(9, 98), mk_evt(8, 97), osd_reqid_t(client, 8, 1))));
+  log.dups.push_back(pg_log_dup_t(mk_ple_mod(mk_obj(1),
+         mk_evt(9, 99), mk_evt(8, 98), osd_reqid_t(client, 8, 1))));
+
+  log.add(mk_ple_mod(mk_obj(1), mk_evt(10, 100), mk_evt(9, 99),
+                    osd_reqid_t(client, 8, 1)));
+  log.add(mk_ple_dt(mk_obj(2), mk_evt(15, 101), mk_evt(10, 100),
+                   osd_reqid_t(client, 8, 2)));
+  log.add(mk_ple_mod_rb(mk_obj(3), mk_evt(15, 102), mk_evt(15, 101),
+                       osd_reqid_t(client, 8, 3)));
+  log.add(mk_ple_mod(mk_obj(1), mk_evt(20, 103), mk_evt(15, 102),
+                    osd_reqid_t(client, 8, 4)));
+  log.add(mk_ple_mod(mk_obj(4), mk_evt(21, 104), mk_evt(20, 103),
+                    osd_reqid_t(client, 8, 5)));
+  log.add(mk_ple_dt_rb(mk_obj(5), mk_evt(21, 105), mk_evt(21, 104),
+                      osd_reqid_t(client, 8, 6)));
+  log.add(mk_ple_dt_rb(mk_obj(5), mk_evt(21, 106), mk_evt(21, 105),
+                      osd_reqid_t(client, 8, 6)));
+  log.add(mk_ple_dt_rb(mk_obj(5), mk_evt(21, 107), mk_evt(21, 106),
+                      osd_reqid_t(client, 8, 6)));
+
+  eversion_t write_from_dups = eversion_t::max();
+
+  log.trim(cct, mk_evt(20, 102), nullptr, nullptr, &write_from_dups);
+
+  EXPECT_EQ(eversion_t::max(), write_from_dups) << log;
+  EXPECT_EQ(5u, log.log.size()) << log;
+  EXPECT_EQ(0u, log.dups.size()) << log;
 }
 
 // This tests copy_up_to() to make copies of

diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc
index 2d8637c4ed86f15fdea16da694e60c9fd757f527..b51c7c1f5dc24e6cce5712166ba8629844eef9ce 100644 (file)
--- a/src/tools/ceph_objectstore_tool.cc
+++ b/src/tools/ceph_objectstore_tool.cc
@@ -447,7 +447,7 @@ int get_log(ObjectStore *fs, __u8 struct_ver,
     ostringstream oss;
     ceph_assert(struct_ver > 0);
     PGLog::read_log_and_missing(
-      fs, ch,
+      g_ceph_context, fs, ch,
       pgid.make_pgmeta_oid(),
       info, log, missing,
       oss,