DO NOT MERGE TO MAIN

Tightens logic around reporting of when multiple set OIs are called in a single transaction. Also tidies up interfaces for log_stats function and makes log messages more consistent.

Intentionally left unsigned to flag up in GitHub actions if it ends up in main.

Jon

diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index f805c0f9776976b56dcae0804f2ec4638e94efab..cd2c9be3957089d494f5f1ef3bfdda5bb2fa6ab9 100644 (file)
--- a/src/osd/ECBackend.cc
+++ b/src/osd/ECBackend.cc
@@ -398,7 +398,7 @@ void ECBackend::handle_sub_write(
   if (!get_parent()->pgb_is_primary())
   {
     get_parent()->update_stats(op.stats);
-    get_parent()->log_stats(op.soid, op.stats.stats.sum, false, op.t);
+    get_parent()->log_stats(op.soid, op.stats.stats.sum, op.t, false);
   }
   ObjectStore::Transaction localt;
   if (!op.temp_added.empty()) {

diff --git a/src/osd/ECCommon.cc b/src/osd/ECCommon.cc
index eeaea2d429d096f6f6eee04d7ac473b447aef871..f9db5fe9aeda532aee00c81892398b85cc7d102f 100644 (file)
--- a/src/osd/ECCommon.cc
+++ b/src/osd/ECCommon.cc
@@ -791,8 +791,8 @@ void ECCommon::RMWPipeline::cache_ready(Op &op) {
 
   get_parent()->log_stats(op.hoid,
                           op.delta_stats,
-                          true,
-                          trans[get_parent()->whoami_shard().shard]);
+                          trans[get_parent()->whoami_shard().shard],
+                          true);
 
   dout(20) << __func__ << ": written: " << written << ", op: " << op << dendl;
 

diff --git a/src/osd/ECListener.h b/src/osd/ECListener.h
index eee2999596532c997781be90bb64459ff50ca2d7..e4b7443569c39f9dd45df7805d97b58690b06dba 100644 (file)
--- a/src/osd/ECListener.h
+++ b/src/osd/ECListener.h
@@ -168,9 +168,9 @@ struct ECListener {
      const object_stat_sum_t &delta_stats) = 0;
 
   virtual void log_stats(hobject_t soid,
-                         object_stat_sum_t stats,
-                         bool delta,
-                         ObjectStore::Transaction& t) = 0;
+                         const object_stat_sum_t& stats,
+                         ObjectStore::Transaction& t,
+                         bool is_delta) = 0;
 
   // new batch
   virtual bool is_missing_object(const hobject_t& oid) const = 0;

diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
index 5009c6cbd0081fb1df926b6c649965ef1997456a..81e15e956c07bb15002c68c020050a2a7e272c5a 100644 (file)
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -116,9 +116,9 @@ typedef std::shared_ptr<const OSDMap> OSDMapRef;
        const hobject_t &soid,
        const object_stat_sum_t &delta_stats) = 0;
      virtual void log_stats(hobject_t soid,
-                            object_stat_sum_t stats,
-                            bool delta,
-                            ObjectStore::Transaction& t) = 0;
+                            const object_stat_sum_t& stats,
+                            ObjectStore::Transaction& t,
+                            bool is_delta) = 0;
 
      /**
       * Called when a read from a std::set of replicas/primary fails

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 320a2fb3d5c7e6507bfa126f36c11d500e6a50df..cdc5f434c75d4e537dedbce3e1afea55c74a8a6d 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -9160,14 +9160,14 @@ void PrimaryLogPG::finish_ctx(OpContext *ctx, int log_op_type, int result)
 }
 
 void PrimaryLogPG::log_stats(hobject_t soid,
-                             object_stat_sum_t stats,
-                             bool delta,
-                             ObjectStore::Transaction& t)
+                             const object_stat_sum_t& stats,
+                             ObjectStore::Transaction& t,
+                             bool is_delta)
 {
   Formatter *f = Formatter::create("json");
 
   std::string operation = "updated to ";
-  if (delta)
+  if (is_delta)
   {
     operation = "delta applied ";
   }
@@ -9180,7 +9180,7 @@ void PrimaryLogPG::log_stats(hobject_t soid,
   f->flush(*_dout);
   *_dout << dendl;
 
-  int set_oi_count = 0;
+  std::map<hobject_t, int> soid_oi_set_count_map;
 
   ObjectStore::Transaction::iterator i = t.begin();
 
@@ -9212,8 +9212,6 @@ void PrimaryLogPG::log_stats(hobject_t soid,
 
     case ObjectStore::Transaction::OP_SETATTR:
       {
-        set_oi_count++;
-
         string name = i.decode_string();
         if (name == OI_ATTR)
         {
@@ -9224,10 +9222,12 @@ void PrimaryLogPG::log_stats(hobject_t soid,
           oi.dump(f);
           f->close_section();
 
-          dout(20) << __func__ << ", soid: " << soid << "."
+          dout(20) << __func__ << ", soid: " << soid
                    << " setattr - OI set to ";
           f->flush(*_dout);
-          *_dout << "bl: " << bl << dendl;
+          *_dout << dendl;
+
+          soid_oi_set_count_map[soid]++;
         }
       }
       break;
@@ -9251,6 +9251,8 @@ void PrimaryLogPG::log_stats(hobject_t soid,
                      << ". setattrs - OI set to ";
             f->flush(*_dout);
             *_dout << "bl: " << bl << dendl;
+
+            soid_oi_set_count_map[soid]++;
           }
         }
       }
@@ -9289,16 +9291,19 @@ void PrimaryLogPG::log_stats(hobject_t soid,
     }
   }
 
-  if (set_oi_count > 1)
+  for (const auto&[soid, oi_set_count] : soid_oi_set_count_map)
   {
-    f->open_object_section("t");
-    t.dump(f);
-    f->close_section();
-    dout(10) << __func__ << ", soid: " << soid
-             << ". WARNING: oi set multiple ("
-             << set_oi_count << ") times in transaction ";
-    f->flush(*_dout);
-    *_dout << dendl;
+    if (oi_set_count > 1)
+    {
+      f->open_object_section("t");
+      t.dump(f);
+      f->close_section();
+      dout(10) << __func__ << ", soid: " << soid
+               << ". INFO: oi set multiple ("
+               << oi_set_count << ") times in transaction ";
+      f->flush(*_dout);
+      *_dout << dendl;
+    }
   }
 }
 

diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index 3cc56152339e8863015b08a0b90414482ef4c444..8ea7114cff0cc26de9ad30e8dfa574938466cfa2 100644 (file)
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -343,9 +343,9 @@ public:
     const hobject_t &soid,
     const object_stat_sum_t &delta_stats) override;
   void log_stats(hobject_t soid,
-                 object_stat_sum_t stats,
-                 bool delta,
-                 ObjectStore::Transaction& t) override;
+                 const object_stat_sum_t& stats,
+                 ObjectStore::Transaction& t,
+                 bool is_delta) override;
 
   bool primary_error(const hobject_t& soid, eversion_t v);