SERVER-19439 implement ConnectionHook API in NetworkInterfaceImpl

9 files changed, 134 insertions, 81 deletions

diff --git a/src/mongo/executor/network_connection_hook.h b/src/mongo/executor/network_connection_hook.h
new file mode 100644
index 00000000000..6537dab7443
--- /dev/null
+++ b/src/mongo/executor/network_connection_hook.h
@@ -0,0 +1,112 @@
+/**
+ *    Copyright (C) 2015 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.
+ */
+
+#pragma once
+
+#include <boost/optional.hpp>
+
+namespace mongo {
+
+class Status;
+template <typename T>
+class StatusWith;
+struct HostAndPort;
+
+namespace executor {
+
+struct RemoteCommandResponse;
+struct RemoteCommandRequest;
+
+/**
+ * An hooking interface for augmenting an implementation of NetworkInterface with domain-specific
+ * host validation and post-connection logic.
+ */
+class NetworkConnectionHook {
+public:
+    virtual ~NetworkConnectionHook() = default;
+
+    /**
+     * Runs optional validation logic on an isMaster reply from a remote host. If a non-OK
+     * Status is returned, it will be propagated up to the completion handler for the command
+     * that initiated the request that caused this connection to be created. This will
+     * be called once for each connection that is created, even if a remote host with the
+     * same HostAndPort has already successfully passed validation on a different connection.
+     *
+     * This method may be called on a different thread from the caller of startCommand that caused
+     * this connection to be created.
+     *
+     * This method must not throw any exceptions or block on network or disk-IO. However, in the
+     * event that an exception escapes, the NetworkInterface is responsible for calling
+     * std::terminate.
+     */
+    virtual Status validateHost(const HostAndPort& remoteHost,
+                                const RemoteCommandResponse& isMasterReply) = 0;
+
+    /**
+     * Generates a command to run on the remote host immediately after connecting to it.
+     * If a non-OK StatusWith is returned, it will be propagated up to the completion handler
+     * for the command that initiated the request that caused this connection to be created.
+     *
+     * The command will be run after socket setup, SSL handshake, authentication, and wire
+     * protocol detection, but before any commands submitted to the NetworkInterface via
+     * startCommand are run. In the case that it isn't neccessary to run a command, makeRequest
+     * may return boost::none.
+     *
+     * This method may be called on a different thread from the caller of startCommand that caused
+     * this connection to be created.
+     *
+     * This method must not throw any exceptions or block on network or disk-IO. However, in the
+     * event that an exception escapes, the NetworkInterface is responsible for calling
+     * std::terminate.
+     */
+    virtual StatusWith<boost::optional<RemoteCommandRequest>> makeRequest(
+        const HostAndPort& remoteHost) = 0;
+
+    /**
+     * Handles a remote server's reply to the command generated with makeRequest. If a
+     * non-OK Status is returned, it will be propagated up to the completion handler for the
+     * command that initiated the request that caused this connection to be created.
+     *
+     * If the corresponding earlier call to makeRequest for this connection returned
+     * boost::none, the NetworkInterface will not call handleReply.
+     *
+     * This method may be called on a different thread from the caller of startCommand that caused
+     * this connection to be created.
+     *
+     * This method must not throw any exceptions or block on network or disk-IO. However, in the
+     * event that an exception escapes, the NetworkInterface is responsible for calling
+     * std::terminate.
+     */
+    virtual Status handleReply(const HostAndPort& remoteHost, RemoteCommandResponse&& response) = 0;
+
+protected:
+    NetworkConnectionHook() = default;
+};
+
+}  // namespace executor
+}  // namespace mongo
diff --git a/src/mongo/executor/network_interface.h b/src/mongo/executor/network_interface.h
index 6d715db20d0..4eeb6db1d8f 100644
--- a/src/mongo/executor/network_interface.h
+++ b/src/mongo/executor/network_interface.h
@@ -54,64 +54,6 @@ public:
     virtual ~NetworkInterface();
 
     /**
-     * An interface for augmenting the NetworkInterface with domain-specific host validation and
-     * post-connection logic.
-     */
-    class ConnectionHook {
-    public:
-        virtual ~ConnectionHook() = default;
-
-        /**
-         * Runs optional validation logic on an isMaster reply from a remote host. If a non-OK
-         * Status is returned, it will be propagated up to the completion handler for the command
-         * that initiated the request that caused this connection to be created. This will
-         * be called once for each connection that is created, even if a remote host with the
-         * same HostAndPort has already successfully passed validation on a different connection.
-         *
-         * This method must not throw any exceptions or block on network or disk-IO. However, in the
-         * event that an exception escapes, the NetworkInterface is responsible for calling
-         * std::terminate.
-         */
-        virtual Status validateHost(const HostAndPort& remoteHost,
-                                    const RemoteCommandResponse& isMasterReply) = 0;
-
-        /**
-         * Generates a command to run on the remote host immediately after connecting to it.
-         * If a non-OK StatusWith is returned, it will be propagated up to the completion handler
-         * for the command that initiated the request that caused this connection to be created.
-         *
-         * The command will be run after socket setup, SSL handshake, authentication, and wire
-         * protocol detection, but before any commands submitted to the NetworkInterface via
-         * startCommand are run. In the case that it isn't neccessary to run a command, makeRequest
-         * may return boost::none.
-         *
-         * This method must not throw any exceptions or block on network or disk-IO. However, in the
-         * event that an exception escapes, the NetworkInterface is responsible for calling
-         * std::terminate.
-         */
-        virtual StatusWith<boost::optional<RemoteCommandRequest>> makeRequest(
-            const HostAndPort& remoteHost) = 0;
-
-        /**
-         * Handles a remote server's reply to the command generated with makeRequest. If a
-         * non-OK Status is returned, it will be propagated up to the completion handler for the
-         * command that initiated the request that caused this connection to be created.
-         *
-         * If the corresponding earlier call to makeRequest for this connection returned
-         * boost::none, the NetworkInterface will not call handleReply.
-         *
-         * This method must not throw any exceptions or block on network or disk-IO. However, in the
-         * event that an exception escapes, the NetworkInterface is responsible for calling
-         * std::terminate.
-         */
-        virtual Status handleReply(const HostAndPort& remoteHost,
-                                   RemoteCommandResponse&& response) = 0;
-
-    protected:
-        ConnectionHook() = default;
-    };
-
-    /**
      * Returns diagnostic info.
      */
     virtual std::string getDiagnosticString() = 0;
@@ -147,13 +89,6 @@ public:
     virtual void waitForWorkUntil(Date_t when) = 0;
 
     /**
-     * Sets a connection hook for this NetworkInterface. This method can only be
-     * called once, and must be called before startup() or the result
-     * is undefined.
-     */
-    virtual void setConnectionHook(std::unique_ptr<ConnectionHook> hook) = 0;
-
-    /**
      * Signals to the network interface that there is new work (such as a signaled event) for
      * the executor to process.  Wakes the executor from waitForWork() and friends.
      */
diff --git a/src/mongo/executor/network_interface_asio.cpp b/src/mongo/executor/network_interface_asio.cpp
index 768621e681a..9ef278971af 100644
--- a/src/mongo/executor/network_interface_asio.cpp
+++ b/src/mongo/executor/network_interface_asio.cpp
@@ -109,10 +109,6 @@ void NetworkInterfaceASIO::waitForWorkUntil(Date_t when) {
     _isExecutorRunnable = false;
 }
 
-void NetworkInterfaceASIO::setConnectionHook(std::unique_ptr<ConnectionHook> hook) {
-    MONGO_UNREACHABLE;
-}
-
 void NetworkInterfaceASIO::signalWorkAvailable() {
     stdx::unique_lock<stdx::mutex> lk(_executorMutex);
     _signalWorkAvailable_inlock();
diff --git a/src/mongo/executor/network_interface_asio.h b/src/mongo/executor/network_interface_asio.h
index b10120d2d9e..b73a1a36a6a 100644
--- a/src/mongo/executor/network_interface_asio.h
+++ b/src/mongo/executor/network_interface_asio.h
@@ -58,6 +58,8 @@
 namespace mongo {
 namespace executor {
 
+class NetworkConnectionHook;
+
 /**
  * A two-way stream supporting asynchronous reads and writes.
  */
@@ -95,7 +97,6 @@ public:
     void shutdown() override;
     void waitForWork() override;
     void waitForWorkUntil(Date_t when) override;
-    void setConnectionHook(std::unique_ptr<ConnectionHook> hook) override;
     void signalWorkAvailable() override;
     Date_t now() override;
     void startCommand(const TaskExecutor::CallbackHandle& cbHandle,
diff --git a/src/mongo/executor/network_interface_impl.cpp b/src/mongo/executor/network_interface_impl.cpp
index 7f63d5f25da..7835754130e 100644
--- a/src/mongo/executor/network_interface_impl.cpp
+++ b/src/mongo/executor/network_interface_impl.cpp
@@ -35,7 +35,9 @@
 #include <memory>
 
 #include "mongo/client/connection_pool.h"
+#include "mongo/executor/network_connection_hook.h"
 #include "mongo/rpc/get_status_from_command_result.h"
+#include "mongo/stdx/memory.h"
 #include "mongo/stdx/thread.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/log.h"
@@ -63,7 +65,12 @@ ThreadPool::Options makeOptions() {
 
 }  // namespace
 
-NetworkInterfaceImpl::NetworkInterfaceImpl() : NetworkInterface(), _pool(makeOptions()) {}
+NetworkInterfaceImpl::NetworkInterfaceImpl() : NetworkInterfaceImpl(nullptr){};
+
+NetworkInterfaceImpl::NetworkInterfaceImpl(std::unique_ptr<NetworkConnectionHook> hook)
+    : NetworkInterface(),
+      _pool(makeOptions()),
+      _commandRunner(kMessagingPortKeepOpen, std::move(hook)) {}
 
 NetworkInterfaceImpl::~NetworkInterfaceImpl() {}
 
@@ -131,10 +138,6 @@ void NetworkInterfaceImpl::waitForWorkUntil(Date_t when) {
     _isExecutorRunnable = false;
 }
 
-void NetworkInterfaceImpl::setConnectionHook(std::unique_ptr<ConnectionHook> hook) {
-    MONGO_UNREACHABLE;
-}
-
 void NetworkInterfaceImpl::_runOneCommand() {
     stdx::unique_lock<stdx::mutex> lk(_mutex);
     if (_pending.empty()) {
diff --git a/src/mongo/executor/network_interface_impl.h b/src/mongo/executor/network_interface_impl.h
index 63a71f1150a..f9c9ce0b856 100644
--- a/src/mongo/executor/network_interface_impl.h
+++ b/src/mongo/executor/network_interface_impl.h
@@ -43,6 +43,8 @@
 namespace mongo {
 namespace executor {
 
+class NetworkConnectionHook;
+
 /**
  * Implementation of the network interface for use by classes implementing TaskExecutor
  * inside mongod.
@@ -72,13 +74,13 @@ namespace executor {
 class NetworkInterfaceImpl final : public NetworkInterface {
 public:
     NetworkInterfaceImpl();
+    NetworkInterfaceImpl(std::unique_ptr<NetworkConnectionHook> hook);
     ~NetworkInterfaceImpl();
     std::string getDiagnosticString() override;
     void startup() override;
     void shutdown() override;
     void waitForWork() override;
     void waitForWorkUntil(Date_t when) override;
-    void setConnectionHook(std::unique_ptr<ConnectionHook> hook) override;
     void signalWorkAvailable() override;
     Date_t now() override;
     std::string getHostName() override;
@@ -153,7 +155,7 @@ private:
     bool _inShutdown = false;
 
     // Interface for running remote commands
-    RemoteCommandRunnerImpl _commandRunner{kMessagingPortKeepOpen};
+    RemoteCommandRunnerImpl _commandRunner;
 
     // Number of active network requests
     size_t _numActiveNetworkRequests = 0;
diff --git a/src/mongo/executor/network_interface_mock.cpp b/src/mongo/executor/network_interface_mock.cpp
index 4e8c427d12e..6bb52d829d8 100644
--- a/src/mongo/executor/network_interface_mock.cpp
+++ b/src/mongo/executor/network_interface_mock.cpp
@@ -31,6 +31,7 @@
 #include "mongo/platform/basic.h"
 
 #include "mongo/executor/network_interface_mock.h"
+#include "mongo/executor/network_connection_hook.h"
 
 #include <algorithm>
 #include <iterator>
@@ -390,7 +391,7 @@ void NetworkInterfaceMock::_connectThenEnqueueOperation_inlock(const HostAndPort
     _enqueueOperation_inlock(std::move(postconnectOp));
 }
 
-void NetworkInterfaceMock::setConnectionHook(std::unique_ptr<ConnectionHook> hook) {
+void NetworkInterfaceMock::setConnectionHook(std::unique_ptr<NetworkConnectionHook> hook) {
     stdx::lock_guard<stdx::mutex> lk(_mutex);
     invariant(!_hasStarted);
     invariant(!_hook);
diff --git a/src/mongo/executor/network_interface_mock.h b/src/mongo/executor/network_interface_mock.h
index f8fc61840f8..4a7da1a6b0a 100644
--- a/src/mongo/executor/network_interface_mock.h
+++ b/src/mongo/executor/network_interface_mock.h
@@ -44,6 +44,8 @@
 namespace mongo {
 namespace executor {
 
+class NetworkConnectionHook;
+
 /**
  * Mock network implementation for use in unit tests.
  *
@@ -85,7 +87,7 @@ public:
     virtual void shutdown();
     virtual void waitForWork();
     virtual void waitForWorkUntil(Date_t when);
-    virtual void setConnectionHook(std::unique_ptr<ConnectionHook> hook);
+    virtual void setConnectionHook(std::unique_ptr<NetworkConnectionHook> hook);
     virtual void signalWorkAvailable();
     virtual Date_t now();
     virtual std::string getHostName();
@@ -301,7 +303,7 @@ private:
     std::priority_queue<AlarmInfo, std::vector<AlarmInfo>, std::greater<AlarmInfo>> _alarms;  // (M)
 
     // The connection hook.
-    std::unique_ptr<ConnectionHook> _hook;  // (R)
+    std::unique_ptr<NetworkConnectionHook> _hook;  // (R)
 
     // The set of hosts we have seen so far. If we see a new host, we will execute the
     // ConnectionHook's validation and post-connection logic.
diff --git a/src/mongo/executor/network_interface_mock_test.cpp b/src/mongo/executor/network_interface_mock_test.cpp
index 8e623778458..011e0f0924e 100644
--- a/src/mongo/executor/network_interface_mock_test.cpp
+++ b/src/mongo/executor/network_interface_mock_test.cpp
@@ -34,6 +34,7 @@
 
 #include "mongo/base/status.h"
 #include "mongo/executor/network_interface.h"
+#include "mongo/executor/network_connection_hook.h"
 #include "mongo/executor/network_interface_mock.h"
 #include "mongo/executor/thread_pool_mock.h"
 #include "mongo/stdx/memory.h"
@@ -44,7 +45,7 @@ namespace executor {
 namespace {
 
 template <typename ValidateFunc, typename RequestFunc, typename ReplyFunc>
-class TestConnectionHook final : public NetworkInterface::ConnectionHook {
+class TestConnectionHook final : public NetworkConnectionHook {
 public:
     TestConnectionHook(ValidateFunc&& validateFunc,
                        RequestFunc&& requestFunc,