From 111849b4d28898a464c63f771336b6edd874559d 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 67ebc97674c..4bba157caaa 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 600f61b7a8f..ad1bfe6c131 100644 --- a/src/osd/ECCommon.cc +++ b/src/osd/ECCommon.cc @@ -844,8 +844,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 10b45aa52f8..43e1031d0c3 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 789e0a20b81..070e41fb342 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -9162,14 +9162,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 "; } @@ -9182,7 +9182,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(); @@ -9214,8 +9214,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) { @@ -9226,10 +9224,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; @@ -9253,6 +9253,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]++; } } } @@ -9291,16 +9293,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 4e193f5c792..bdaa2309071 100644 --- a/src/osd/PrimaryLogPG.h +++ b/src/osd/PrimaryLogPG.h @@ -344,9 +344,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