SERVER-29152 Do not cache logging ostream in threadlocal when in other thread-specific contexts

4 files changed, 48 insertions, 19 deletions

diff --git a/src/mongo/client/connpool.cpp b/src/mongo/client/connpool.cpp
index 5b72068a0e3..598d58afcd1 100644
--- a/src/mongo/client/connpool.cpp
+++ b/src/mongo/client/connpool.cpp
@@ -63,8 +63,8 @@ PoolForHost::~PoolForHost() {
 
 void PoolForHost::clear() {
     if (!_parentDestroyed) {
-        log() << "Dropping all pooled connections to " << _hostName << "(with timeout of "
-              << _socketTimeout << " seconds)";
+        logNoCache() << "Dropping all pooled connections to " << _hostName << "(with timeout of "
+                     << _socketTimeout << " seconds)";
     }
 
     while (!_pool.empty()) {
@@ -86,18 +86,18 @@ void PoolForHost::done(DBConnectionPool* pool, DBClientBase* c) {
     if (isFailed ||
         // Another (later) connection was reported as broken to this host
         (c->getSockCreationMicroSec() < _minValidCreationTimeMicroSec)) {
-        log() << "Ending connection to host " << _hostName << "(with timeout of " << _socketTimeout
-              << " seconds)"
-              << " due to bad connection status; " << openConnections()
-              << " connections to that host remain open";
+        logNoCache() << "Ending connection to host " << _hostName << "(with timeout of "
+                     << _socketTimeout << " seconds)"
+                     << " due to bad connection status; " << openConnections()
+                     << " connections to that host remain open";
         pool->onDestroy(c);
         delete c;
     } else if (_maxPoolSize >= 0 && static_cast<int>(_pool.size()) >= _maxPoolSize) {
         // We have a pool size that we need to enforce
-        log() << "Ending idle connection to host " << _hostName << "(with timeout of "
-              << _socketTimeout << " seconds)"
-              << " because the pool meets constraints; " << openConnections()
-              << " connections to that host remain open";
+        logNoCache() << "Ending idle connection to host " << _hostName << "(with timeout of "
+                     << _socketTimeout << " seconds)"
+                     << " because the pool meets constraints; " << openConnections()
+                     << " connections to that host remain open";
         pool->onDestroy(c);
         delete c;
     } else {
@@ -111,9 +111,9 @@ void PoolForHost::reportBadConnectionAt(uint64_t microSec) {
         microSec > _minValidCreationTimeMicroSec) {
         _minValidCreationTimeMicroSec = microSec;
 
-        log() << "Detected bad connection created at " << _minValidCreationTimeMicroSec
-              << " microSec, clearing pool for " << _hostName << " of " << openConnections()
-              << " connections" << endl;
+        logNoCache() << "Detected bad connection created at " << _minValidCreationTimeMicroSec
+                     << " microSec, clearing pool for " << _hostName << " of " << openConnections()
+                     << " connections" << endl;
 
         clear();
     }
diff --git a/src/mongo/logger/logstream_builder.cpp b/src/mongo/logger/logstream_builder.cpp
index 2577704aad0..199991c7dd7 100644
--- a/src/mongo/logger/logstream_builder.cpp
+++ b/src/mongo/logger/logstream_builder.cpp
@@ -81,12 +81,14 @@ LogstreamBuilder::LogstreamBuilder(MessageLogDomain* domain,
 LogstreamBuilder::LogstreamBuilder(MessageLogDomain* domain,
                                    std::string contextName,
                                    LogSeverity severity,
-                                   LogComponent component)
+                                   LogComponent component,
+                                   bool shouldCache)
     : _domain(domain),
       _contextName(std::move(contextName)),
       _severity(std::move(severity)),
       _component(std::move(component)),
-      _tee(nullptr) {}
+      _tee(nullptr),
+      _shouldCache(shouldCache) {}
 
 LogstreamBuilder::LogstreamBuilder(logger::MessageLogDomain* domain,
                                    const std::string& contextName,
@@ -103,7 +105,8 @@ LogstreamBuilder::LogstreamBuilder(LogstreamBuilder&& other)
       _baseMessage(std::move(other._baseMessage)),
       _os(std::move(other._os)),
       _tee(std::move(other._tee)),
-      _isTruncatable(other._isTruncatable) {}
+      _isTruncatable(other._isTruncatable),
+      _shouldCache(other._shouldCache) {}
 
 LogstreamBuilder& LogstreamBuilder::operator=(LogstreamBuilder&& other) {
     _domain = std::move(other._domain);
@@ -114,6 +117,7 @@ LogstreamBuilder& LogstreamBuilder::operator=(LogstreamBuilder&& other) {
     _os = std::move(other._os);
     _tee = std::move(other._tee);
     _isTruncatable = std::move(other._isTruncatable);
+    _shouldCache = other._shouldCache;
     return *this;
 }
 
@@ -134,7 +138,8 @@ LogstreamBuilder::~LogstreamBuilder() {
             _tee->write(_os->str());
         }
         _os->str("");
-        if (isThreadOstreamCacheInitialized && !threadOstreamCache.getMake()->get()) {
+        if (_shouldCache && isThreadOstreamCacheInitialized &&
+            !threadOstreamCache.getMake()->get()) {
             *threadOstreamCache.get() = std::move(_os);
         }
     }
@@ -148,7 +153,8 @@ void LogstreamBuilder::operator<<(Tee* tee) {
 
 void LogstreamBuilder::makeStream() {
     if (!_os) {
-        if (isThreadOstreamCacheInitialized && threadOstreamCache.getMake()->get()) {
+        if (_shouldCache && isThreadOstreamCacheInitialized &&
+            threadOstreamCache.getMake()->get()) {
             _os = std::move(*threadOstreamCache.get());
         } else {
             _os = stdx::make_unique<std::ostringstream>();
diff --git a/src/mongo/logger/logstream_builder.h b/src/mongo/logger/logstream_builder.h
index 40f56ccf75f..f829621c8a7 100644
--- a/src/mongo/logger/logstream_builder.h
+++ b/src/mongo/logger/logstream_builder.h
@@ -72,11 +72,17 @@ public:
      * "contextName" is a short name of the thread or other context.
      * "severity" is the logging severity of the message.
      * "component" is the primary log component of the message.
+     *
+     * By default, this class will create one ostream per thread, and it
+     * will cache that object in a threadlocal and reuse it for subsequent
+     * logs messages. Set "shouldCache" to false to create a new ostream
+     * for each instance of this class rather than cacheing.
      */
     LogstreamBuilder(MessageLogDomain* domain,
                      std::string contextName,
                      LogSeverity severity,
-                     LogComponent component);
+                     LogComponent component,
+                     bool shouldCache = true);
 
     /**
      * Deprecated.
@@ -236,6 +242,7 @@ private:
     std::unique_ptr<std::ostringstream> _os;
     Tee* _tee;
     bool _isTruncatable = true;
+    bool _shouldCache;
 };
 
 
diff --git a/src/mongo/util/log.h b/src/mongo/util/log.h
index b0f53a30b87..3f17151d4f4 100644
--- a/src/mongo/util/log.h
+++ b/src/mongo/util/log.h
@@ -132,6 +132,22 @@ inline LogstreamBuilder log() {
                             ::MongoLogDefaultComponent_component);
 }
 
+/**
+ * Returns a LogstreamBuilder that does not cache its ostream in a threadlocal cache.
+ * Use this variant when logging from places that may not be able to access threadlocals,
+ * such as from within other threadlocal-managed objects, or thread_specific_ptr-managed
+ * objects.
+ *
+ * Once SERVER-29377 is completed, this overload can be removed.
+ */
+inline LogstreamBuilder logNoCache() {
+    return LogstreamBuilder(logger::globalLogDomain(),
+                            getThreadName(),
+                            logger::LogSeverity::Log(),
+                            ::MongoLogDefaultComponent_component,
+                            false);
+}
+
 inline LogstreamBuilder log(logger::LogComponent component) {
     return LogstreamBuilder(
         logger::globalLogDomain(), getThreadName(), logger::LogSeverity::Log(), component);