From cfde896f8e67602f7ba1b7e36e608906eb885623 Mon Sep 17 00:00:00 2001 From: Jon Bailey Date: Wed, 16 Jul 2025 13:32:16 +0100 Subject: [PATCH] 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 --- src/osd/ECBackend.cc | 2 +- src/osd/ECCommon.cc | 4 ++-- src/osd/ECListener.h | 6 +++--- src/osd/PGBackend.h | 6 +++--- src/osd/PrimaryLogPG.cc | 41 +++++++++++++++++++++++------------------ src/osd/PrimaryLogPG.h | 6 +++--- 6 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc index f805c0f9776..cd2c9be3957 100644 --- 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 eeaea2d429d..f9db5fe9aed 100644 --- 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 eee29995965..e4b7443569c 100644 --- 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 5009c6cbd00..81e15e956c0 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -116,9 +116,9 @@ typedef std::shared_ptr 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 320a2fb3d5c..cdc5f434c75 100644 --- 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 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 3cc56152339..8ea7114cff0 100644 --- 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); -- 2.39.5