From 7b27aa1eae1fd12690073a2c9da2ec35a84a8fd0 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 29 Apr 2019 10:09:39 -0700 Subject: [PATCH] src/osd: refactor to use ostream_temp The concept of emitting log lines to a central mon log is likely to be useful for both crimson and classic, but the mechanism is likely to be different. Break out OstreamTemp for use in interface. Signed-off-by: Samuel Just --- src/common/CMakeLists.txt | 1 + src/common/LogClient.cc | 17 -------------- src/common/LogClient.h | 46 +++++++++++--------------------------- src/common/LogEntry.h | 10 +-------- src/common/ostream_temp.cc | 15 +++++++++++++ src/common/ostream_temp.h | 39 ++++++++++++++++++++++++++++++++ src/osd/PG.cc | 12 ++++++++-- src/osd/PG.h | 4 +++- src/osd/PGBackend.h | 5 +++-- src/osd/PeeringState.cc | 26 ++++++++++----------- src/osd/PeeringState.h | 7 ++++-- src/osd/PrimaryLogPG.h | 4 ++-- 12 files changed, 105 insertions(+), 81 deletions(-) create mode 100644 src/common/ostream_temp.cc create mode 100644 src/common/ostream_temp.h diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 826d6539ac81f..f8c351a10f383 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -20,6 +20,7 @@ set(common_srcs HeartbeatMap.cc LogClient.cc LogEntry.cc + ostream_temp.cc Mutex.cc OutputDataSocket.cc PluginRegistry.cc diff --git a/src/common/LogClient.cc b/src/common/LogClient.cc index 982bbe35b49e9..24aa31841d6e3 100644 --- a/src/common/LogClient.cc +++ b/src/common/LogClient.cc @@ -128,23 +128,6 @@ LogClient::LogClient(CephContext *cct, Messenger *m, MonMap *mm, { } -LogClientTemp::LogClientTemp(clog_type type_, LogChannel &parent_) - : type(type_), parent(parent_) -{ -} - -LogClientTemp::LogClientTemp(const LogClientTemp &rhs) - : type(rhs.type), parent(rhs.parent) -{ - // don't want to-- nor can we-- copy the ostringstream -} - -LogClientTemp::~LogClientTemp() -{ - if (ss.peek() != EOF) - parent.do_log(type, ss); -} - void LogChannel::update_config(map &log_to_monitors, map &log_to_syslog, map &log_channels, diff --git a/src/common/LogClient.h b/src/common/LogClient.h index 64e779a3f61ce..84b41823661ad 100644 --- a/src/common/LogClient.h +++ b/src/common/LogClient.h @@ -17,6 +17,7 @@ #include #include "common/LogEntry.h" +#include "common/ostream_temp.h" #include "common/ceph_mutex.h" #include "include/health.h" @@ -48,25 +49,6 @@ int parse_log_client_options(CephContext *cct, uuid_d &fsid, std::string &host); -class LogClientTemp -{ -public: - LogClientTemp(clog_type type_, LogChannel &parent_); - LogClientTemp(const LogClientTemp &rhs); - ~LogClientTemp(); - - template - std::ostream& operator<<(const T& rhs) - { - return ss << rhs; - } - -private: - clog_type type; - LogChannel &parent; - std::stringstream ss; -}; - /** Manage where we output to and at which priority * * Not to be confused with the LogClient, which is the almighty coordinator @@ -76,7 +58,7 @@ private: * Past queueing the LogEntry, the LogChannel is done with the whole thing. * LogClient will deal with sending and handling of LogEntries. */ -class LogChannel +class LogChannel : public OstreamTemp::OstreamTempSink { public: @@ -86,8 +68,8 @@ public: const std::string &facility, const std::string &prio); - LogClientTemp debug() { - return LogClientTemp(CLOG_DEBUG, *this); + OstreamTemp debug() { + return OstreamTemp(CLOG_DEBUG, this); } void debug(std::stringstream &s) { do_log(CLOG_DEBUG, s); @@ -96,7 +78,7 @@ public: * Convenience function mapping health status to * the appropriate cluster log severity. */ - LogClientTemp health(health_status_t health) { + OstreamTemp health(health_status_t health) { switch(health) { case HEALTH_OK: return info(); @@ -109,26 +91,26 @@ public: ceph_abort(); } } - LogClientTemp info() { - return LogClientTemp(CLOG_INFO, *this); + OstreamTemp info() { + return OstreamTemp(CLOG_INFO, this); } void info(std::stringstream &s) { do_log(CLOG_INFO, s); } - LogClientTemp warn() { - return LogClientTemp(CLOG_WARN, *this); + OstreamTemp warn() { + return OstreamTemp(CLOG_WARN, this); } void warn(std::stringstream &s) { do_log(CLOG_WARN, s); } - LogClientTemp error() { - return LogClientTemp(CLOG_ERROR, *this); + OstreamTemp error() { + return OstreamTemp(CLOG_ERROR, this); } void error(std::stringstream &s) { do_log(CLOG_ERROR, s); } - LogClientTemp sec() { - return LogClientTemp(CLOG_SEC, *this); + OstreamTemp sec() { + return OstreamTemp(CLOG_SEC, this); } void sec(std::stringstream &s) { do_log(CLOG_SEC, s); @@ -200,8 +182,6 @@ private: bool log_to_monitors; std::shared_ptr graylog; - - friend class LogClientTemp; }; typedef LogChannel::Ref LogChannelRef; diff --git a/src/common/LogEntry.h b/src/common/LogEntry.h index 21134ef42b095..3ef600b900a17 100644 --- a/src/common/LogEntry.h +++ b/src/common/LogEntry.h @@ -18,20 +18,12 @@ #include "include/utime.h" #include "msg/msg_types.h" #include "common/entity_name.h" +#include "ostream_temp.h" namespace ceph { class Formatter; } -typedef enum { - CLOG_DEBUG = 0, - CLOG_INFO = 1, - CLOG_SEC = 2, - CLOG_WARN = 3, - CLOG_ERROR = 4, - CLOG_UNKNOWN = -1, -} clog_type; - static const std::string CLOG_CHANNEL_NONE = "none"; static const std::string CLOG_CHANNEL_DEFAULT = "cluster"; static const std::string CLOG_CHANNEL_CLUSTER = "cluster"; diff --git a/src/common/ostream_temp.cc b/src/common/ostream_temp.cc new file mode 100644 index 0000000000000..61ae5b741ccd4 --- /dev/null +++ b/src/common/ostream_temp.cc @@ -0,0 +1,15 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "common/ostream_temp.h" + +OstreamTemp::OstreamTemp(clog_type type_, OstreamTempSink *parent_) + : type(type_), parent(parent_) +{ +} + +OstreamTemp::~OstreamTemp() +{ + if (ss.peek() != EOF && parent) + parent->do_log(type, ss); +} diff --git a/src/common/ostream_temp.h b/src/common/ostream_temp.h new file mode 100644 index 0000000000000..722b189cd31ff --- /dev/null +++ b/src/common/ostream_temp.h @@ -0,0 +1,39 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#pragma once + +#include + +typedef enum { + CLOG_DEBUG = 0, + CLOG_INFO = 1, + CLOG_SEC = 2, + CLOG_WARN = 3, + CLOG_ERROR = 4, + CLOG_UNKNOWN = -1, +} clog_type; + +class OstreamTemp +{ +public: + class OstreamTempSink { + public: + virtual void do_log(clog_type prio, std::stringstream& ss) = 0; + virtual ~OstreamTempSink() {} + }; + OstreamTemp(clog_type type_, OstreamTempSink *parent_); + OstreamTemp(OstreamTemp &&rhs) = default; + ~OstreamTemp(); + + template + std::ostream& operator<<(const T& rhs) + { + return ss << rhs; + } + +private: + clog_type type; + OstreamTempSink *parent; + std::stringstream ss; +}; diff --git a/src/osd/PG.cc b/src/osd/PG.cc index e849863cdfc44..61ef4e4e4a3ed 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -1625,8 +1625,16 @@ epoch_t PG::oldest_stored_osdmap() { return osd->get_superblock().oldest_map; } -LogChannel &PG::get_clog() { - return *(osd->clog); +OstreamTemp PG::get_clog_info() { + return osd->clog->info(); +} + +OstreamTemp PG::get_clog_debug() { + return osd->clog->debug(); +} + +OstreamTemp PG::get_clog_error() { + return osd->clog->error(); } void PG::schedule_event_after( diff --git a/src/osd/PG.h b/src/osd/PG.h index 44bfe82033c17..f2e651e7bd35a 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -410,7 +410,9 @@ public: void clear_primary_state() override; epoch_t oldest_stored_osdmap() override; - LogChannel &get_clog() override; + OstreamTemp &get_clog_error() override; + OstreamTemp &get_clog_info() override; + OstreamTemp &get_clog_debug() override; void schedule_event_after( PGPeeringEventRef event, diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h index d8fca3277e05f..70c01ac075893 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -25,6 +25,7 @@ #include "common/LogClient.h" #include #include "PGTransaction.h" +#include "common/ostream_temp.h" namespace Scrub { class Store; @@ -287,8 +288,8 @@ typedef std::shared_ptr OSDMapRef; virtual ceph_tid_t get_tid() = 0; - virtual LogClientTemp clog_error() = 0; - virtual LogClientTemp clog_warn() = 0; + virtual OstreamTemp clog_error() = 0; + virtual OstreamTemp clog_warn() = 0; virtual bool check_failsafe_full() = 0; diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc index ab19bccfa4b7c..23020c9532676 100644 --- a/src/osd/PeeringState.cc +++ b/src/osd/PeeringState.cc @@ -840,7 +840,7 @@ void PeeringState::check_past_interval_bounds() const pl->oldest_stored_osdmap()); if (rpib.first >= rpib.second) { if (!past_intervals.empty()) { - pl->get_clog().error() << info.pgid << " required past_interval bounds are" + pl->get_clog_error() << info.pgid << " required past_interval bounds are" << " empty [" << rpib << ") but past_intervals is not: " << past_intervals; derr << info.pgid << " required past_interval bounds are" @@ -849,7 +849,7 @@ void PeeringState::check_past_interval_bounds() const } } else { if (past_intervals.empty()) { - pl->get_clog().error() << info.pgid << " required past_interval bounds are" + pl->get_clog_error() << info.pgid << " required past_interval bounds are" << " not empty [" << rpib << ") but past_intervals " << past_intervals << " is empty"; derr << info.pgid << " required past_interval bounds are" @@ -860,7 +860,7 @@ void PeeringState::check_past_interval_bounds() const auto apib = past_intervals.get_bounds(); if (apib.first > rpib.first) { - pl->get_clog().error() << info.pgid << " past_intervals [" << apib + pl->get_clog_error() << info.pgid << " past_intervals [" << apib << ") start interval does not contain the required" << " bound [" << rpib << ") start"; derr << info.pgid << " past_intervals [" << apib @@ -869,7 +869,7 @@ void PeeringState::check_past_interval_bounds() const ceph_abort_msg("past_interval start interval mismatch"); } if (apib.second != rpib.second) { - pl->get_clog().error() << info.pgid << " past_interal bound [" << apib + pl->get_clog_error() << info.pgid << " past_interal bound [" << apib << ") end does not match required [" << rpib << ") end"; derr << info.pgid << " past_interal bound [" << apib @@ -1922,18 +1922,18 @@ bool PeeringState::choose_acting(pg_shard_t &auth_log_shard_id, void PeeringState::log_weirdness() { if (pg_log.get_tail() != info.log_tail) - pl->get_clog().error() << info.pgid + pl->get_clog_error() << info.pgid << " info mismatch, log.tail " << pg_log.get_tail() << " != info.log_tail " << info.log_tail; if (pg_log.get_head() != info.last_update) - pl->get_clog().error() << info.pgid + pl->get_clog_error() << info.pgid << " info mismatch, log.head " << pg_log.get_head() << " != info.last_update " << info.last_update; if (!pg_log.get_log().empty()) { // sloppy check if ((pg_log.get_log().log.begin()->version <= pg_log.get_tail())) - pl->get_clog().error() << info.pgid + pl->get_clog_error() << info.pgid << " log bound mismatch, info (tail,head] (" << pg_log.get_tail() << "," << pg_log.get_head() << "]" @@ -1943,7 +1943,7 @@ void PeeringState::log_weirdness() } if (pg_log.get_log().caller_ops.size() > pg_log.get_log().log.size()) { - pl->get_clog().error() << info.pgid + pl->get_clog_error() << info.pgid << " caller_ops.size " << pg_log.get_log().caller_ops.size() << " > log size " << pg_log.get_log().log.size(); @@ -2211,7 +2211,7 @@ void PeeringState::activate( if (pi.last_update == info.last_update && !force_restart_backfill) { // empty log if (!pi.last_backfill.is_max()) - pl->get_clog().info() << info.pgid << " continuing backfill to osd." + pl->get_clog_info() << info.pgid << " continuing backfill to osd." << peer << " from (" << pi.log_tail << "," << pi.last_update << "] " << pi.last_backfill @@ -2247,7 +2247,7 @@ void PeeringState::activate( * behind. */ // backfill - pl->get_clog().debug() << info.pgid << " starting backfill to osd." << peer + pl->get_clog_debug() << info.pgid << " starting backfill to osd." << peer << " from (" << pi.log_tail << "," << pi.last_update << "] " << pi.last_backfill << " to " << info.last_update; @@ -2576,7 +2576,7 @@ void PeeringState::fulfill_log( psdout(10) << " sending info+missing+log since " << query.since << dendl; if (query.since != eversion_t() && query.since < pg_log.get_tail()) { - pl->get_clog().error() << info.pgid << " got broken pg_query_t::LOG since " + pl->get_clog_error() << info.pgid << " got broken pg_query_t::LOG since " << query.since << " when my log.tail is " << pg_log.get_tail() << ", sending full log instead"; @@ -5377,12 +5377,12 @@ boost::statechart::result PeeringState::Active::react(const ActMap&) if (unfound > 0 && ps->all_unfound_are_queried_or_lost(ps->get_osdmap())) { if (ps->cct->_conf->osd_auto_mark_unfound_lost) { - pl->get_clog().error() << context< PeeringMachine >().spgid.pgid << " has " << unfound + pl->get_clog_error() << context< PeeringMachine >().spgid.pgid << " has " << unfound << " objects unfound and apparently lost, would automatically " << "mark these objects lost but this feature is not yet implemented " << "(osd_auto_mark_unfound_lost)"; } else - pl->get_clog().error() << context< PeeringMachine >().spgid.pgid << " has " + pl->get_clog_error() << context< PeeringMachine >().spgid.pgid << " has " << unfound << " objects unfound and apparently lost"; } diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h index f53b09e40a836..55d9a554fc542 100644 --- a/src/osd/PeeringState.h +++ b/src/osd/PeeringState.h @@ -22,8 +22,8 @@ #include "os/ObjectStore.h" #include "OSDMap.h" #include "MissingLoc.h" -#include "common/LogClient.h" #include "osd/osd_perf_counters.h" +#include "common/ostream_temp.h" struct PGPool { CephContext* cct; @@ -218,7 +218,10 @@ public: const char *state_name, utime_t enter_time, uint64_t events, utime_t event_dur) = 0; virtual void dump_recovery_info(Formatter *f) const = 0; - virtual LogChannel &get_clog() = 0; + + virtual OstreamTemp get_clog_info() = 0; + virtual OstreamTemp get_clog_error() = 0; + virtual OstreamTemp get_clog_debug() = 0; virtual ~PeeringListener() {} }; diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h index 33a0d32b2236c..e91937e33b92e 100644 --- a/src/osd/PrimaryLogPG.h +++ b/src/osd/PrimaryLogPG.h @@ -527,8 +527,8 @@ public: ceph_tid_t get_tid() override { return osd->get_tid(); } - LogClientTemp clog_error() override { return osd->clog->error(); } - LogClientTemp clog_warn() override { return osd->clog->warn(); } + OstreamTemp clog_error() override { return osd->clog->error(); } + OstreamTemp clog_warn() override { return osd->clog->warn(); } struct watch_disconnect_t { uint64_t cookie; -- 2.39.5