SERVER-31548 ServiceExecutor should be started before TransportLayer

6 files changed, 182 insertions, 11 deletions

diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 5e1b55f9dee..dc651e91c7b 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -820,15 +820,15 @@ ExitCode _initAndListen(int listenPort) {
     // operation context anymore
     startupOpCtx.reset();
 
-    auto start = serviceContext->getTransportLayer()->start();
+    auto start = serviceContext->getServiceExecutor()->start();
     if (!start.isOK()) {
-        error() << "Failed to start the listener: " << start.toString();
+        error() << "Failed to start the service executor: " << start;
         return EXIT_NET_ERROR;
     }
 
-    start = serviceContext->getServiceExecutor()->start();
+    start = serviceContext->getTransportLayer()->start();
     if (!start.isOK()) {
-        error() << "Failed to start the service executor: " << start;
+        error() << "Failed to start the listener: " << start.toString();
         return EXIT_NET_ERROR;
     }
 
diff --git a/src/mongo/s/server.cpp b/src/mongo/s/server.cpp
index 0af7f2a2288..aa28c33d81f 100644
--- a/src/mongo/s/server.cpp
+++ b/src/mongo/s/server.cpp
@@ -447,14 +447,14 @@ static ExitCode runMongosServer() {
     // Set up the logical session cache
     LogicalSessionCache::set(getGlobalServiceContext(), makeLogicalSessionCacheS());
 
-    auto start = getGlobalServiceContext()->getTransportLayer()->start();
+    auto start = getGlobalServiceContext()->getServiceExecutor()->start();
     if (!start.isOK()) {
+        error() << "Failed to start the service executor: " << start;
         return EXIT_NET_ERROR;
     }
 
-    start = getGlobalServiceContext()->getServiceExecutor()->start();
+    start = getGlobalServiceContext()->getTransportLayer()->start();
     if (!start.isOK()) {
-        error() << "Failed to start the service executor: " << start;
         return EXIT_NET_ERROR;
     }
 
diff --git a/src/mongo/transport/SConscript b/src/mongo/transport/SConscript
index a4ca32867d6..60d055c5efa 100644
--- a/src/mongo/transport/SConscript
+++ b/src/mongo/transport/SConscript
@@ -90,6 +90,19 @@ tlEnv.Library(
     ],
 )
 
+tlEnv.CppUnitTest(
+    target='service_executor_test',
+    source=[
+        'service_executor_test.cpp',
+    ],
+    LIBDEPS=[
+        'service_executor',
+        '$BUILD_DIR/mongo/db/service_context',
+        '$BUILD_DIR/mongo/unittest/unittest',
+        '$BUILD_DIR/third_party/shim_asio',
+    ],
+)
+
 # Disable this test until SERVER-30475 and associated build failure tickets
 # are resolved.
 #
diff --git a/src/mongo/transport/service_executor_adaptive.cpp b/src/mongo/transport/service_executor_adaptive.cpp
index 03e34de92e6..dbc91ed4c5f 100644
--- a/src/mongo/transport/service_executor_adaptive.cpp
+++ b/src/mongo/transport/service_executor_adaptive.cpp
@@ -190,6 +190,10 @@ Status ServiceExecutorAdaptive::schedule(ServiceExecutorAdaptive::Task task, Sch
     auto pendingCounterPtr = (flags & kDeferredTask) ? &_deferredTasksQueued : &_tasksQueued;
     pendingCounterPtr->addAndFetch(1);
 
+    if (!_isRunning.load()) {
+        return {ErrorCodes::ShutdownInProgress, "Executor is not running"};
+    }
+
     auto wrappedTask = [ this, task = std::move(task), scheduleTime, pendingCounterPtr ] {
         pendingCounterPtr->subtractAndFetch(1);
         auto start = _tickSource->getTicks();
diff --git a/src/mongo/transport/service_executor_synchronous.cpp b/src/mongo/transport/service_executor_synchronous.cpp
index 8c66e42744e..45ad6a95536 100644
--- a/src/mongo/transport/service_executor_synchronous.cpp
+++ b/src/mongo/transport/service_executor_synchronous.cpp
@@ -89,12 +89,16 @@ Status ServiceExecutorSynchronous::shutdown(Milliseconds timeout) {
 }
 
 Status ServiceExecutorSynchronous::schedule(Task task, ScheduleFlags flags) {
+    if (!_stillRunning.load()) {
+        return Status{ErrorCodes::ShutdownInProgress, "Executor is not running"};
+    }
+
     if (!_localWorkQueue.empty()) {
         /*
-        * In perf testing we found that yielding after running a each request produced
-        * at 5% performance boost in microbenchmarks if the number of worker threads
-        * was greater than the number of available cores.
-        */
+         * In perf testing we found that yielding after running a each request produced
+         * at 5% performance boost in microbenchmarks if the number of worker threads
+         * was greater than the number of available cores.
+         */
         if (flags & ScheduleFlags::kMayYieldBeforeSchedule) {
             if ((_localThreadIdleCounter++ & 0xf) == 0) {
                 markThreadIdle();
diff --git a/src/mongo/transport/service_executor_test.cpp b/src/mongo/transport/service_executor_test.cpp
new file mode 100644
index 00000000000..7134ce21e45
--- /dev/null
+++ b/src/mongo/transport/service_executor_test.cpp
@@ -0,0 +1,150 @@
+/**
+ *    Copyright (C) 2017 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or  modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kDefault;
+
+#include "mongo/platform/basic.h"
+
+#include "boost/optional.hpp"
+
+#include "mongo/db/service_context_noop.h"
+#include "mongo/transport/service_executor_adaptive.h"
+#include "mongo/transport/service_executor_synchronous.h"
+#include "mongo/unittest/unittest.h"
+#include "mongo/util/log.h"
+#include "mongo/util/scopeguard.h"
+
+#include <asio.hpp>
+
+namespace mongo {
+namespace {
+using namespace transport;
+
+struct TestOptions : public ServiceExecutorAdaptive::Options {
+    int reservedThreads() const final {
+        return 1;
+    }
+
+    Milliseconds workerThreadRunTime() const final {
+        return Milliseconds{1000};
+    }
+
+    int runTimeJitter() const final {
+        return 0;
+    }
+
+    Milliseconds stuckThreadTimeout() const final {
+        return Milliseconds{100};
+    }
+
+    Microseconds maxQueueLatency() const final {
+        return duration_cast<Microseconds>(Milliseconds{5});
+    }
+
+    int idlePctThreshold() const final {
+        return 0;
+    }
+
+    int recursionLimit() const final {
+        return 0;
+    }
+};
+
+class ServiceExecutorAdaptiveFixture : public unittest::Test {
+protected:
+    void setUp() override {
+        auto scOwned = stdx::make_unique<ServiceContextNoop>();
+        setGlobalServiceContext(std::move(scOwned));
+        asioIOCtx = std::make_shared<asio::io_context>();
+
+        auto configOwned = stdx::make_unique<TestOptions>();
+        executorConfig = configOwned.get();
+        executor = stdx::make_unique<ServiceExecutorAdaptive>(
+            getGlobalServiceContext(), asioIOCtx, std::move(configOwned));
+    }
+
+    ServiceExecutorAdaptive::Options* executorConfig;
+    std::unique_ptr<ServiceExecutorAdaptive> executor;
+    std::shared_ptr<asio::io_context> asioIOCtx;
+};
+
+class ServiceExecutorSynchronousFixture : public unittest::Test {
+protected:
+    void setUp() override {
+        auto scOwned = stdx::make_unique<ServiceContextNoop>();
+        setGlobalServiceContext(std::move(scOwned));
+
+        executor = stdx::make_unique<ServiceExecutorSynchronous>(getGlobalServiceContext());
+    }
+
+    std::unique_ptr<ServiceExecutorSynchronous> executor;
+};
+
+void scheduleBasicTask(ServiceExecutor* exec, bool expectSuccess) {
+    stdx::condition_variable cond;
+    stdx::mutex mutex;
+    auto task = [&cond, &mutex] {
+        stdx::unique_lock<stdx::mutex> lk(mutex);
+        cond.notify_all();
+    };
+
+    stdx::unique_lock<stdx::mutex> lk(mutex);
+    auto status = exec->schedule(std::move(task), ServiceExecutor::kEmptyFlags);
+    if (expectSuccess) {
+        ASSERT_OK(status);
+        cond.wait(lk);
+    } else {
+        ASSERT_NOT_OK(status);
+    }
+}
+
+TEST_F(ServiceExecutorAdaptiveFixture, BasicTaskRuns) {
+    ASSERT_OK(executor->start());
+    auto guard = MakeGuard([this] { ASSERT_OK(executor->shutdown(Milliseconds{500})); });
+
+    scheduleBasicTask(executor.get(), true);
+}
+
+TEST_F(ServiceExecutorAdaptiveFixture, ScheduleFailsBeforeStartup) {
+    scheduleBasicTask(executor.get(), false);
+}
+
+TEST_F(ServiceExecutorSynchronousFixture, BasicTaskRuns) {
+    ASSERT_OK(executor->start());
+    auto guard = MakeGuard([this] { ASSERT_OK(executor->shutdown(Milliseconds{500})); });
+
+    scheduleBasicTask(executor.get(), true);
+}
+
+TEST_F(ServiceExecutorSynchronousFixture, ScheduleFailsBeforeStartup) {
+    scheduleBasicTask(executor.get(), false);
+}
+
+
+}  // namespace
+}  // namespace mongo