From: NitzanMordhai <nmordech@redhat.com>
Date: Mon, 21 Mar 2022 11:34:34 +0000 (+0000)
Subject: osd/PGLog.cc: Trim duplicates by number of entries
X-Git-Tag: v17.2.1~45^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3ff0df6a28a1d9e197bdba40be7126fed8a14ae9;p=ceph.git

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 0f184497ada9..3fa711a6f1dc 100644
--- 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 06d0024743a9..34ed71a72b36 100644
--- 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 b08b9f0ccc28..004e3cd1c589 100644
--- 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 2d8637c4ed86..b51c7c1f5dc2 100644
--- 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,