SERVER-67927 ServiceExecutor: ThreadingModel -> HasDedicatedThread

5 files changed, 72 insertions, 179 deletions

diff --git a/src/mongo/transport/service_entry_point_impl.cpp b/src/mongo/transport/service_entry_point_impl.cpp
index 524dc94a964..386f0e9f851 100644
--- a/src/mongo/transport/service_entry_point_impl.cpp
+++ b/src/mongo/transport/service_entry_point_impl.cpp
@@ -306,7 +306,7 @@ void ServiceEntryPointImpl::startSession(transport::SessionHandle session) {
         // Imbue the new Client with a ServiceExecutorContext.
         {
             auto seCtx = std::make_unique<transport::ServiceExecutorContext>();
-            seCtx->setThreadingModel(transport::ServiceExecutor::getInitialThreadingModel());
+            seCtx->setUseDedicatedThread(transport::gInitialUseDedicatedThread);
             seCtx->setCanUseReserved(isPrivilegedSession);
             stdx::lock_guard lk(*client);
             transport::ServiceExecutorContext::set(&*client, std::move(seCtx));
diff --git a/src/mongo/transport/service_executor.cpp b/src/mongo/transport/service_executor.cpp
index b61c55ea250..747ea46e477 100644
--- a/src/mongo/transport/service_executor.cpp
+++ b/src/mongo/transport/service_executor.cpp
@@ -32,7 +32,10 @@
 
 #include "mongo/transport/service_executor.h"
 
+#include <algorithm>
+#include <array>
 #include <boost/optional.hpp>
+#include <utility>
 
 #include "mongo/logv2/log.h"
 #include "mongo/transport/service_entry_point.h"
@@ -45,52 +48,22 @@
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kNetwork
 
 
-namespace mongo {
-namespace transport {
-namespace {
-static constexpr auto kDiagnosticLogLevel = 4;
+namespace mongo::transport {
 
-static constexpr auto kThreadingModelDedicatedStr = "dedicated"_sd;
-static constexpr auto kThreadingModelBorrowedStr = "borrowed"_sd;
+bool gInitialUseDedicatedThread = true;
 
-auto gInitialThreadingModel = ServiceExecutor::ThreadingModel::kDedicated;
+namespace {
+static constexpr auto kDiagnosticLogLevel = 4;
 
 auto getServiceExecutorStats =
     ServiceContext::declareDecoration<synchronized_value<ServiceExecutorStats>>();
 auto getServiceExecutorContext =
     Client::declareDecoration<std::unique_ptr<ServiceExecutorContext>>();
-}  // namespace
-
-StringData toString(ServiceExecutor::ThreadingModel threadingModel) {
-    switch (threadingModel) {
-        case ServiceExecutor::ThreadingModel::kDedicated:
-            return kThreadingModelDedicatedStr;
-        case ServiceExecutor::ThreadingModel::kBorrowed:
-            return kThreadingModelBorrowedStr;
-        default:
-            MONGO_UNREACHABLE;
-    }
-}
-
-Status ServiceExecutor::setInitialThreadingModelFromString(StringData value) noexcept {
-    if (value == kThreadingModelDedicatedStr) {
-        setInitialThreadingModel(ServiceExecutor::ThreadingModel::kDedicated);
-    } else if (value == kThreadingModelBorrowedStr) {
-        setInitialThreadingModel(ServiceExecutor::ThreadingModel::kBorrowed);
-    } else {
-        MONGO_UNREACHABLE;
-    }
-
-    return Status::OK();
-}
-
-void ServiceExecutor::setInitialThreadingModel(ThreadingModel threadingModel) noexcept {
-    gInitialThreadingModel = threadingModel;
-}
 
-auto ServiceExecutor::getInitialThreadingModel() noexcept -> ThreadingModel {
-    return gInitialThreadingModel;
+void incrThreadingModelStats(ServiceExecutorStats& stats, bool useDedicatedThread, int step) {
+    (useDedicatedThread ? stats.usesDedicated : stats.usesBorrowed) += step;
 }
+}  // namespace
 
 ServiceExecutorStats ServiceExecutorStats::get(ServiceContext* ctx) noexcept {
     return getServiceExecutorStats(ctx).get();
@@ -111,97 +84,46 @@ void ServiceExecutorContext::set(Client* client,
     seCtx._sep = client->getServiceContext()->getServiceEntryPoint();
 
     {
-        auto stats = getServiceExecutorStats(client->getServiceContext()).synchronize();
-        if (seCtx._canUseReserved) {
-            ++stats->limitExempt;
-        }
-
-        switch (seCtx._threadingModel) {
-            case ThreadingModel::kBorrowed: {
-                ++stats->usesBorrowed;
-            } break;
-            case ThreadingModel::kDedicated: {
-                ++stats->usesDedicated;
-            } break;
-            default:
-                MONGO_UNREACHABLE;
-        }
+        auto&& syncStats = *getServiceExecutorStats(client->getServiceContext());
+        if (seCtx._canUseReserved)
+            ++syncStats->limitExempt;
+        incrThreadingModelStats(*syncStats, seCtx._useDedicatedThread, 1);
     }
 
     LOGV2_DEBUG(4898000,
                 kDiagnosticLogLevel,
                 "Setting initial ServiceExecutor context for client",
                 "client"_attr = client->desc(),
-                "threadingModel"_attr = seCtx._threadingModel,
+                "useDedicatedThread"_attr = seCtx._useDedicatedThread,
                 "canUseReserved"_attr = seCtx._canUseReserved);
     serviceExecutorContext = std::move(seCtxPtr);
 }
 
 void ServiceExecutorContext::reset(Client* client) noexcept {
-    if (client) {
-        auto& serviceExecutorContext = getServiceExecutorContext(client);
-
-        auto stats = getServiceExecutorStats(client->getServiceContext()).synchronize();
-
-        LOGV2_DEBUG(4898001,
-                    kDiagnosticLogLevel,
-                    "Resetting ServiceExecutor context for client",
-                    "client"_attr = client->desc(),
-                    "threadingModel"_attr = serviceExecutorContext->_threadingModel,
-                    "canUseReserved"_attr = serviceExecutorContext->_canUseReserved);
-
-        if (serviceExecutorContext->_canUseReserved) {
-            --stats->limitExempt;
-        }
-
-        switch (serviceExecutorContext->_threadingModel) {
-            case ThreadingModel::kBorrowed: {
-                --stats->usesBorrowed;
-            } break;
-            case ThreadingModel::kDedicated: {
-                --stats->usesDedicated;
-            } break;
-            default:
-                MONGO_UNREACHABLE;
-        }
-    }
+    if (!client)
+        return;
+    auto& seCtx = getServiceExecutorContext(client);
+    LOGV2_DEBUG(4898001,
+                kDiagnosticLogLevel,
+                "Resetting ServiceExecutor context for client",
+                "client"_attr = client->desc(),
+                "threadingModel"_attr = seCtx->_useDedicatedThread,
+                "canUseReserved"_attr = seCtx->_canUseReserved);
+    auto stats = *getServiceExecutorStats(client->getServiceContext());
+    if (seCtx->_canUseReserved)
+        --stats->limitExempt;
+    incrThreadingModelStats(*stats, seCtx->_useDedicatedThread, -1);
 }
 
-void ServiceExecutorContext::setThreadingModel(ThreadingModel threadingModel) noexcept {
-
-    if (_threadingModel == threadingModel) {
-        // Nothing to do.
+void ServiceExecutorContext::setUseDedicatedThread(bool b) noexcept {
+    if (b == _useDedicatedThread)
         return;
-    }
-
-    auto lastThreadingModel = std::exchange(_threadingModel, threadingModel);
-
-    if (_client) {
-        auto stats = getServiceExecutorStats(_client->getServiceContext()).synchronize();
-
-        // Decrement the stats for the previous ThreadingModel.
-        switch (lastThreadingModel) {
-            case ThreadingModel::kBorrowed: {
-                --stats->usesBorrowed;
-            } break;
-            case ThreadingModel::kDedicated: {
-                --stats->usesDedicated;
-            } break;
-            default:
-                MONGO_UNREACHABLE;
-        }
-        // Increment the stats for the next ThreadingModel.
-        switch (_threadingModel) {
-            case ThreadingModel::kBorrowed: {
-                ++stats->usesBorrowed;
-            } break;
-            case ThreadingModel::kDedicated: {
-                ++stats->usesDedicated;
-            } break;
-            default:
-                MONGO_UNREACHABLE;
-        }
-    }
+    auto prev = std::exchange(_useDedicatedThread, b);
+    if (!_client)
+        return;
+    auto stats = *getServiceExecutorStats(_client->getServiceContext());
+    incrThreadingModelStats(*stats, prev, -1);
+    incrThreadingModelStats(*stats, _useDedicatedThread, +1);
 }
 
 void ServiceExecutorContext::setCanUseReserved(bool canUseReserved) noexcept {
@@ -223,16 +145,8 @@ void ServiceExecutorContext::setCanUseReserved(bool canUseReserved) noexcept {
 
 ServiceExecutor* ServiceExecutorContext::getServiceExecutor() noexcept {
     invariant(_client);
-
-    switch (_threadingModel) {
-        case ThreadingModel::kBorrowed:
-            return ServiceExecutorFixed::get(_client->getServiceContext());
-        case ThreadingModel::kDedicated: {
-            // Continue on.
-        } break;
-        default:
-            MONGO_UNREACHABLE;
-    }
+    if (!_useDedicatedThread)
+        return ServiceExecutorFixed::get(_client->getServiceContext());
 
     auto shouldUseReserved = [&] {
         // This is at best a naive solution. There could be a world where numOpenSessions() changes
@@ -292,5 +206,4 @@ void ServiceExecutor::shutdownAll(ServiceContext* serviceContext, Date_t deadlin
     }
 }
 
-}  // namespace transport
-}  // namespace mongo
+}  // namespace mongo::transport
diff --git a/src/mongo/transport/service_executor.h b/src/mongo/transport/service_executor.h
index 56e43b4c404..458f152bd89 100644
--- a/src/mongo/transport/service_executor.h
+++ b/src/mongo/transport/service_executor.h
@@ -45,26 +45,13 @@
 namespace mongo {
 namespace transport {
 
+extern bool gInitialUseDedicatedThread;
+
 /*
  * This is the interface for all ServiceExecutors.
  */
 class ServiceExecutor : public OutOfLineExecutor {
 public:
-    /**
-     * An enum to indicate if a ServiceExecutor should use dedicated or borrowed threading
-     * resources.
-     */
-    enum class ThreadingModel {
-        kBorrowed,
-        kDedicated,
-    };
-
-    friend StringData toString(ThreadingModel threadingModel);
-
-    static Status setInitialThreadingModelFromString(StringData value) noexcept;
-    static void setInitialThreadingModel(ThreadingModel threadingModel) noexcept;
-    static ThreadingModel getInitialThreadingModel() noexcept;
-
     static void shutdownAll(ServiceContext* serviceContext, Date_t deadline);
 
     virtual ~ServiceExecutor() = default;
@@ -142,8 +129,6 @@ public:
  */
 class ServiceExecutorContext {
 public:
-    using ThreadingModel = ServiceExecutor::ThreadingModel;
-
     /**
      * Get a pointer to the ServiceExecutorContext for a given client.
      *
@@ -173,11 +158,11 @@ public:
     ServiceExecutorContext& operator=(ServiceExecutorContext&&) = delete;
 
     /**
-     * Set the ThreadingModel for the associated Client's service execution.
+     * Set the threading model for the associated Client's service execution.
      *
      * This function is only valid to invoke with the Client lock or before the Client is set.
      */
-    void setThreadingModel(ThreadingModel threadingModel) noexcept;
+    void setUseDedicatedThread(bool dedicated) noexcept;
 
     /**
      * Set if reserved resources are available for the associated Client's service execution.
@@ -191,8 +176,8 @@ public:
      *
      * This function is valid to invoke either on the Client thread or with the Client lock.
      */
-    auto getThreadingModel() const noexcept {
-        return _threadingModel;
+    bool useDedicatedThread() const noexcept {
+        return _useDedicatedThread;
     }
 
     /**
@@ -207,7 +192,7 @@ private:
     Client* _client = nullptr;
     ServiceEntryPoint* _sep = nullptr;
 
-    ThreadingModel _threadingModel = ThreadingModel::kDedicated;
+    bool _useDedicatedThread = true;
     bool _canUseReserved = false;
     bool _hasUsedSynchronous = false;
 };
diff --git a/src/mongo/transport/service_executor.idl b/src/mongo/transport/service_executor.idl
index 1758e45ee13..8471f2eebeb 100644
--- a/src/mongo/transport/service_executor.idl
+++ b/src/mongo/transport/service_executor.idl
@@ -32,14 +32,13 @@ global:
     - "mongo/transport/service_executor.h"
 
 server_parameters:
-  initialServiceExecutorThreadingModel:
+  initialServiceExecutorUseDedicatedThread:
     description: >-
-        Start new client connections using an executor that follows this model.
+        If true, each client will use a dedicated thread.
     set_at: [ startup ]
-    cpp_vartype: "std::string"
-    cpp_varname: "initialServiceExecutorThreadingModel"
-    on_update: "ServiceExecutor::setInitialThreadingModelFromString"
-    default: "dedicated"
+    cpp_vartype: bool
+    cpp_varname: gInitialServiceExecutorUseDedicatedThread
+    default: true
   synchronousServiceExecutorRecursionLimit:
     description: >-
         Tasks may recurse further if their recursion depth is less than this value.
diff --git a/src/mongo/transport/service_state_machine_test.cpp b/src/mongo/transport/service_state_machine_test.cpp
index 5d742ca81da..ad01830adec 100644
--- a/src/mongo/transport/service_state_machine_test.cpp
+++ b/src/mongo/transport/service_state_machine_test.cpp
@@ -65,6 +65,21 @@ namespace mongo {
 namespace transport {
 namespace {
 
+/** Scope guard to set and restore an object value. */
+template <typename T>
+class ScopedValueOverride {
+public:
+    ScopedValueOverride(T& target, T v)
+        : _target{target}, _saved{std::exchange(_target, std::move(v))} {}
+    ~ScopedValueOverride() {
+        _target = std::move(_saved);
+    }
+
+private:
+    T& _target;
+    T _saved;
+};
+
 // TODO(cr) Does this go into its own header?
 /**
  * The ServiceStateMachineTest is a fixture that mocks the external inputs into the
@@ -151,9 +166,6 @@ public:
         return std::make_shared<ThreadPool>(std::move(options));
     }
 
-    ServiceStateMachineTest(boost::optional<ServiceExecutor::ThreadingModel> threadingModel = {})
-        : _threadingModel(std::move(threadingModel)) {}
-
     void setUp() override;
     void tearDown() override;
 
@@ -463,9 +475,6 @@ private:
         ++_onClientDisconnectCalled;
     }
 
-    const boost::optional<ServiceExecutor::ThreadingModel> _threadingModel;
-    boost::optional<ServiceExecutor::ThreadingModel> _originalThreadingModel;
-
     ServiceStateMachineTest::ServiceEntryPoint* _sep;
 
     const std::shared_ptr<ThreadPool> _threadPool = makeThreadPool();
@@ -754,11 +763,6 @@ void ServiceStateMachineTest::terminateViaServiceEntryPoint() {
 void ServiceStateMachineTest::setUp() {
     ServiceContextTest::setUp();
 
-    if (_threadingModel) {
-        _originalThreadingModel = ServiceExecutor::getInitialThreadingModel();
-        ServiceExecutor::setInitialThreadingModel(*_threadingModel);
-    }
-
     auto sep = std::make_unique<ServiceStateMachineTest::ServiceEntryPoint>(this);
     _sep = sep.get();
     getServiceContext()->setServiceEntryPoint(std::move(sep));
@@ -779,23 +783,15 @@ void ServiceStateMachineTest::tearDown() {
 
     _threadPool->shutdown();
     _threadPool->join();
-
-    if (_originalThreadingModel) {
-        ServiceExecutor::setInitialThreadingModel(*_originalThreadingModel);
-    }
 }
 
-class ServiceStateMachineWithDedicatedThreadsTest : public ServiceStateMachineTest {
-public:
-    ServiceStateMachineWithDedicatedThreadsTest()
-        : ServiceStateMachineTest(ServiceExecutor::ThreadingModel::kDedicated) {}
+template <bool useDedicatedThread>
+class DedicatedThreadOverrideTest : public ServiceStateMachineTest {
+    ScopedValueOverride<bool> _svo{gInitialUseDedicatedThread, useDedicatedThread};
 };
 
-class ServiceStateMachineWithBorrowedThreadsTest : public ServiceStateMachineTest {
-public:
-    ServiceStateMachineWithBorrowedThreadsTest()
-        : ServiceStateMachineTest(ServiceExecutor::ThreadingModel::kBorrowed) {}
-};
+using ServiceStateMachineWithBorrowedThreadsTest = DedicatedThreadOverrideTest<false>;
+using ServiceStateMachineWithDedicatedThreadsTest = DedicatedThreadOverrideTest<true>;
 
 TEST_F(ServiceStateMachineTest, StartThenEndSession) {
     initNewSession();