From 63d27f8eec2b071724613bd923098c95a025e603 Mon Sep 17 00:00:00 2001 From: Giacomo Govi Date: Wed, 16 Dec 2020 10:03:35 +0100 Subject: [PATCH 1/2] Bug fix for coral MessageService wrapper --- CondCore/CondDB/interface/ConnectionPool.h | 3 +- CondCore/CondDB/interface/Logger.h | 10 ++- CondCore/CondDB/src/ConnectionPool.cc | 13 ++- CondCore/CondDB/src/CoralMsgReporter.cc | 85 +++++++++++-------- CondCore/CondDB/src/CoralMsgReporter.h | 27 ++++-- CondCore/CondDB/src/Logger.cc | 26 +++--- .../src/PoolDBOutputService.cc | 2 +- 7 files changed, 99 insertions(+), 67 deletions(-) diff --git a/CondCore/CondDB/interface/ConnectionPool.h b/CondCore/CondDB/interface/ConnectionPool.h index f291d9cf2ba11..620a7ea08be2f 100644 --- a/CondCore/CondDB/interface/ConnectionPool.h +++ b/CondCore/CondDB/interface/ConnectionPool.h @@ -64,14 +64,13 @@ namespace cond { std::string m_authPath = std::string(""); int m_authSys = 0; coral::MsgLevel m_messageLevel = coral::Error; - std::unique_ptr m_msgReporter; + CoralMsgReporter* m_msgReporter = nullptr; bool m_loggingEnabled = false; //The frontier security option is turned on for all sessions //usig this wrapper of the CORAL connection setup for configuring the server access std::string m_frontierSecurity = std::string(""); // this one has to be moved! cond::CoralServiceManager* m_pluginManager = nullptr; - std::map m_dbTypes; }; } // namespace persistency } // namespace cond diff --git a/CondCore/CondDB/interface/Logger.h b/CondCore/CondDB/interface/Logger.h index 399db18216d2d..5a6f3015414be 100644 --- a/CondCore/CondDB/interface/Logger.h +++ b/CondCore/CondDB/interface/Logger.h @@ -14,13 +14,14 @@ #include "FWCore/MessageLogger/interface/MessageLogger.h" // #include +#include #include namespace cond { namespace persistency { - class ConnectionPool; + class MsgDispatcher; template class EchoedLogStream { @@ -93,7 +94,10 @@ namespace cond { virtual ~Logger(); // - void setDbDestination(const std::string& connectionString, ConnectionPool& connectionPool); + void subscribeCoralMessages(const std::weak_ptr& dispatcher); + + // + void setDbDestination(const std::string& connectionString); // void start(); @@ -124,12 +128,12 @@ namespace cond { private: std::string m_jobName; std::string m_connectionString; - ConnectionPool* m_sharedConnectionPool; bool m_started; boost::posix_time::ptime m_startTime; boost::posix_time::ptime m_endTime; int m_retCode; std::stringstream m_log; + std::weak_ptr m_dispatcher; }; } // namespace persistency diff --git a/CondCore/CondDB/src/ConnectionPool.cc b/CondCore/CondDB/src/ConnectionPool.cc index 247bf2d1c841b..ae9f401c2891b 100644 --- a/CondCore/CondDB/src/ConnectionPool.cc +++ b/CondCore/CondDB/src/ConnectionPool.cc @@ -23,6 +23,9 @@ namespace cond { ConnectionPool::ConnectionPool() { m_pluginManager = new cond::CoralServiceManager; + m_msgReporter = new CoralMsgReporter; + // ownership is acquired ( instrinsically unsafe, need to change coral ) + coral::MessageStream::installMsgReporter(m_msgReporter); configure(); } @@ -73,10 +76,8 @@ namespace cond { coralConfig.disableConnectionSharing(); // message streaming coral::MessageStream::setMsgVerbosity(m_messageLevel); - if (m_msgReporter.get() != nullptr) { - m_msgReporter->setOutputLevel(m_messageLevel); - coral::MessageStream::installMsgReporter(static_cast(m_msgReporter.get())); - } + m_msgReporter->setOutputLevel(m_messageLevel); + // authentication std::string authServiceName("CORAL/Services/EnvironmentAuthenticationService"); std::string authPath = m_authPath; @@ -171,9 +172,7 @@ namespace cond { void ConnectionPool::setMessageVerbosity(coral::MsgLevel level) { m_messageLevel = level; } - void ConnectionPool::setLogDestination(Logger& logger) { - m_msgReporter = std::make_unique(logger); - } + void ConnectionPool::setLogDestination(Logger& logger) { m_msgReporter->subscribe(logger); } } // namespace persistency } // namespace cond diff --git a/CondCore/CondDB/src/CoralMsgReporter.cc b/CondCore/CondDB/src/CoralMsgReporter.cc index 603a23be845ae..5eff8d9618ee2 100644 --- a/CondCore/CondDB/src/CoralMsgReporter.cc +++ b/CondCore/CondDB/src/CoralMsgReporter.cc @@ -7,9 +7,17 @@ #include "CondCore/CondDB/interface/Logger.h" #include "CoralMsgReporter.h" +cond::persistency::MsgDispatcher::MsgDispatcher(Logger& logger) { m_recipient = &logger; } + +void cond::persistency::MsgDispatcher::unsubscribe() { m_recipient = nullptr; } + +bool cond::persistency::MsgDispatcher::hasRecipient() { return m_recipient != nullptr; } + +cond::persistency::Logger& cond::persistency::MsgDispatcher::recipient() { return *m_recipient; } + /// Default constructor -cond::persistency::CoralMsgReporter::CoralMsgReporter(Logger& logger) - : m_logger(logger), m_level(coral::Error), m_format(0), m_mutex() { +cond::persistency::CoralMsgReporter::CoralMsgReporter() + : m_dispatcher(), m_level(coral::Error), m_format(0), m_mutex() { // Use a non-default format? //char* msgformat = getenv ( "CORAL_MSGFORMAT" ); if (getenv("CORAL_MESSAGEREPORTER_FORMATTED")) @@ -79,58 +87,61 @@ coral::MsgLevel cond::persistency::CoralMsgReporter::outputLevel() const { retur /// Modify output level void cond::persistency::CoralMsgReporter::setOutputLevel(coral::MsgLevel lvl) { m_level = lvl; } +void reportToRecipient(const std::string& msg, int lvl, cond::persistency::Logger& recipient) { + switch (lvl) { + case coral::Nil: + case coral::Verbose: + case coral::Debug: + recipient.logDebug() << "CORAL: " << msg; + break; + case coral::Info: + recipient.logInfo() << "CORAL: " << msg; + break; + case coral::Warning: + recipient.logWarning() << "CORAL: " << msg; + break; + case coral::Error: + recipient.logError() << "CORAL: " << msg; + break; + } +} + /// Report message to stdout -void cond::persistency::CoralMsgReporter::report(int lvl, const std::string& src, const std::string& msg) { +void cond::persistency::CoralMsgReporter::report(int lvl, const std::string&, const std::string& msg) { if (lvl < m_level) return; std::lock_guard lock(m_mutex); - std::stringstream out; - /** - if ( m_format == 1 ) // COOL format - { - // Formatted CORAL reporter (as in COOL) - //std::ostream& out = std::cout; - const std::string::size_type src_name_maxsize = 36; - if ( src.size() <= src_name_maxsize ) - { - out << src << std::string( src_name_maxsize-src.size(), ' ' ); - } - else - { - out << src.substr( 0, src_name_maxsize-3 ) << "..."; - } - switch ( lvl ) - { - case 0: out << " Nil "; break; - case 1: out << " Verbose "; break; - case 2: out << " Debug "; break; - case 3: out << " Info "; break; - case 4: out << " Warning "; break; - case 5: out << " Error "; break; - case 6: out << " Fatal "; break; - case 7: out << " Always "; break; - default: out << " Unknown "; break; - } - out << msg << std::endl; + if (m_dispatcher.get() && m_dispatcher->hasRecipient()) { + reportToRecipient(msg, lvl, m_dispatcher->recipient()); } - else{ - **/ // Default CORAL reporter + std::string level(""); switch (lvl) { case coral::Nil: + level = "Nil"; + break; case coral::Verbose: + level = "Verbose"; + break; case coral::Debug: - m_logger.logDebug() << "CORAL: " << msg; + level = "Debug"; break; case coral::Info: - m_logger.logInfo() << "CORAL: " << msg; + level = "Info"; break; case coral::Warning: - m_logger.logWarning() << "CORAL: " << msg; + level = "Warning"; break; case coral::Error: - m_logger.logError() << "CORAL: " << msg; + level = "Error"; break; } + std::cout << msg << " " << level << " " << msg << std::endl; +} + +void cond::persistency::CoralMsgReporter::subscribe(Logger& logger) { + m_dispatcher.reset(new MsgDispatcher(logger)); + std::weak_ptr callBack(m_dispatcher); + logger.subscribeCoralMessages(callBack); } diff --git a/CondCore/CondDB/src/CoralMsgReporter.h b/CondCore/CondDB/src/CoralMsgReporter.h index aef1aaa9c2c58..7d51372c58b43 100644 --- a/CondCore/CondDB/src/CoralMsgReporter.h +++ b/CondCore/CondDB/src/CoralMsgReporter.h @@ -4,6 +4,7 @@ //#include "CondCore/CondDB/interface/Logger.h" #include +#include #include "CoralBase/MessageStream.h" namespace cond { @@ -12,16 +13,28 @@ namespace cond { class Logger; - class CoralMsgReporter : public coral::IMsgReporter { + class MsgDispatcher { public: - // Empty ctr is suppressed - CoralMsgReporter() = delete; + MsgDispatcher() = delete; + explicit MsgDispatcher(Logger& logger); + virtual ~MsgDispatcher() {} + + void unsubscribe(); + + bool hasRecipient(); + Logger& recipient(); + private: + Logger* m_recipient = nullptr; + }; + + class CoralMsgReporter : public coral::IMsgReporter { + public: /// Default constructor - explicit CoralMsgReporter(Logger& logger); + CoralMsgReporter(); /// Destructor - ~CoralMsgReporter() override {} + ~CoralMsgReporter() override{}; /// Release reference to reporter void release() override { delete this; } @@ -35,9 +48,11 @@ namespace cond { /// Report a message void report(int lvl, const std::string& src, const std::string& msg) override; + void subscribe(Logger& logger); + private: // the destination of the streams... - Logger& m_logger; + std::shared_ptr m_dispatcher; /// The current message level threshold coral::MsgLevel m_level; diff --git a/CondCore/CondDB/src/Logger.cc b/CondCore/CondDB/src/Logger.cc index 8d6a592e1fa60..e51177948177c 100644 --- a/CondCore/CondDB/src/Logger.cc +++ b/CondCore/CondDB/src/Logger.cc @@ -1,10 +1,13 @@ #include "CondCore/CondDB/interface/Logger.h" -#include "CondCore/CondDB/interface/ConnectionPool.h" +#include "CondCore/CondDB/interface/Auth.h" #include "CondCore/CondDB/interface/Exception.h" // #include "DbCore.h" +#include "CoralMsgReporter.h" #include "RelationalAccess/ITransaction.h" // +#include "RelationalAccess/ConnectionService.h" +#include "RelationalAccess/ISessionProxy.h" #include #include // @@ -45,7 +48,6 @@ namespace cond { Logger::Logger(const std::string& jobName) : m_jobName(jobName), m_connectionString(""), - m_sharedConnectionPool(nullptr), m_started(false), m_startTime(), m_endTime(), @@ -53,13 +55,16 @@ namespace cond { m_log() {} // - Logger::~Logger() {} - - void Logger::setDbDestination(const std::string& connectionString, ConnectionPool& connectionPool) { - m_connectionString = connectionString; - m_sharedConnectionPool = &connectionPool; + Logger::~Logger() { + auto dispatcher = m_dispatcher.lock(); + if (dispatcher.get()) + dispatcher->unsubscribe(); } + void Logger::subscribeCoralMessages(const std::weak_ptr& dispatcher) { m_dispatcher = dispatcher; } + + void Logger::setDbDestination(const std::string& connectionString) { m_connectionString = connectionString; } + // void Logger::start() { if (!m_started) { @@ -114,13 +119,12 @@ namespace cond { // void Logger::saveOnDb() { if (!m_log.str().empty()) { - if (m_sharedConnectionPool == nullptr) { - throwException("Connection pool handle has not been provided.", "Logger::saveOnDb"); - } if (m_connectionString.empty()) { throwException("Connection string for destination database has not been provided.", "Logger::saveOnDb"); } - auto coralSession = m_sharedConnectionPool->createCoralSession(m_connectionString, true); + coral::ConnectionService connServ; + std::unique_ptr coralSession( + connServ.connect(m_connectionString, auth::COND_WRITER_ROLE, coral::Update)); coralSession->transaction().start(false); try { O2O_RUN::Table destinationTable(coralSession->nominalSchema()); diff --git a/CondCore/DBOutputService/src/PoolDBOutputService.cc b/CondCore/DBOutputService/src/PoolDBOutputService.cc index f9e4d4c75a707..c9ab7a07955cf 100644 --- a/CondCore/DBOutputService/src/PoolDBOutputService.cc +++ b/CondCore/DBOutputService/src/PoolDBOutputService.cc @@ -55,7 +55,7 @@ cond::service::PoolDBOutputService::PoolDBOutputService(const edm::ParameterSet& m_session = m_connection.createSession(connectionString, true); bool saveLogsOnDb = iConfig.getUntrackedParameter("saveLogsOnDB", false); if (saveLogsOnDb) - m_logger.setDbDestination(connectionString, m_connection); + m_logger.setDbDestination(connectionString); // implicit start doStartTransaction(); From 5eca006e9055ff7327df353cf9dcb80c9d1ce439 Mon Sep 17 00:00:00 2001 From: Giacomo Govi Date: Thu, 17 Dec 2020 10:26:46 +0100 Subject: [PATCH 2/2] Comment removed --- CondCore/CondDB/src/ConnectionPool.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/CondCore/CondDB/src/ConnectionPool.cc b/CondCore/CondDB/src/ConnectionPool.cc index ae9f401c2891b..3336629b21bf2 100644 --- a/CondCore/CondDB/src/ConnectionPool.cc +++ b/CondCore/CondDB/src/ConnectionPool.cc @@ -24,7 +24,6 @@ namespace cond { ConnectionPool::ConnectionPool() { m_pluginManager = new cond::CoralServiceManager; m_msgReporter = new CoralMsgReporter; - // ownership is acquired ( instrinsically unsafe, need to change coral ) coral::MessageStream::installMsgReporter(m_msgReporter); configure(); }