SERVER-18246 getmore on secondary in recovery mode can crash mongos

6 files changed, 58 insertions, 35 deletions

diff --git a/src/mongo/client/dbclient.cpp b/src/mongo/client/dbclient.cpp
index 12df4b577d9..89d0a814bc5 100644
--- a/src/mongo/client/dbclient.cpp
+++ b/src/mongo/client/dbclient.cpp
@@ -34,8 +34,8 @@
 #include "mongo/bson/util/bson_extract.h"
 #include "mongo/bson/util/builder.h"
 #include "mongo/client/constants.h"
-#include "mongo/client/dbclient_rs.h"
 #include "mongo/client/dbclientcursor.h"
+#include "mongo/client/replica_set_monitor.h"
 #include "mongo/client/sasl_client_authenticate.h"
 #include "mongo/config.h"
 #include "mongo/db/auth/internal_user_auth.h"
@@ -935,15 +935,14 @@ namespace mongo {
                                         int options) {
         if (DBClientWithCommands::runCommand(dbname, cmd, info, options))
             return true;
-        
-        if ( clientSet && isNotMasterErrorString( info["errmsg"] ) ) {
-            clientSet->isntMaster();
+
+        if (!_parentReplSetName.empty()) {
+            handleNotMasterResponse(info["errmsg"]);
         }
 
         return false;
     }
 
-
     void DBClientConnection::_checkConnection() {
         if ( !_failed )
             return;
@@ -1053,10 +1052,6 @@ namespace mongo {
         return n;
     }
 
-    void DBClientConnection::setReplSetClientCallback(DBClientReplicaSet* rsClient) {
-        clientSet = rsClient;
-    }
-
     unsigned long long DBClientConnection::query(
             stdx::function<void(DBClientCursorBatchIterator &)> f,
             const string& ns,
@@ -1393,6 +1388,14 @@ namespace mongo {
         toSend.setData(dbQuery, b.buf(), b.len());
     }
 
+    DBClientConnection::DBClientConnection(bool _autoReconnect, double so_timeout):
+                    _failed(false),
+                    autoReconnect(_autoReconnect),
+                    autoReconnectBackoff(1000, 2000),
+                    _so_timeout(so_timeout) {
+        _numConnections.fetchAndAdd(1);
+    }
+
     void DBClientConnection::say( Message &toSend, bool isRetry , string * actualServer ) {
         checkConnection();
         try {
@@ -1469,12 +1472,10 @@ namespace mongo {
         *retry = false;
         *host = _serverString;
 
-        if ( clientSet && nReturned ) {
+        if (!_parentReplSetName.empty() && nReturned) {
             verify(data);
-            BSONObj o(data);
-            if ( isNotMasterErrorString( getErrField(o) ) ) {
-                clientSet->isntMaster();
-            }
+            BSONObj bsonView(data);
+            handleNotMasterResponse(getErrField(bsonView));
         }
     }
 
@@ -1493,6 +1494,27 @@ namespace mongo {
             say(m);
     }
 
+    void DBClientConnection::setParentReplSetName(const string& replSetName) {
+        _parentReplSetName = replSetName;
+    }
+
+    void DBClientConnection::handleNotMasterResponse(const BSONElement& elemToCheck) {
+        if (!isNotMasterErrorString(elemToCheck)) {
+            return;
+        }
+
+        MONGO_LOG_COMPONENT(1, logger::LogComponent::kReplication)
+            << "got not master from: " << _serverString
+            << " of repl set: " << _parentReplSetName;
+
+        ReplicaSetMonitorPtr monitor = ReplicaSetMonitor::get(_parentReplSetName);
+        if (monitor) {
+            monitor->failedHost(_server);
+        }
+
+        _failed = true;
+    }
+
 #ifdef MONGO_CONFIG_SSL
     static SimpleMutex s_mtx("SSLManager");
     static SSLManagerInterface* s_sslMgr(NULL);
@@ -1512,7 +1534,7 @@ namespace mongo {
 
 
     bool serverAlive( const string &uri ) {
-        DBClientConnection c( false, 0, 20 ); // potentially the connection to server could fail while we're checking if it's alive - so use timeouts
+        DBClientConnection c(false, 20); // potentially the connection to server could fail while we're checking if it's alive - so use timeouts
         string err;
         if ( !c.connect( HostAndPort(uri), err ) )
             return false;
diff --git a/src/mongo/client/dbclient_rs.cpp b/src/mongo/client/dbclient_rs.cpp
index 086eae84215..4f9f5d975e5 100644
--- a/src/mongo/client/dbclient_rs.cpp
+++ b/src/mongo/client/dbclient_rs.cpp
@@ -344,7 +344,7 @@ namespace {
 
         _masterHost = h;
         _master.reset(newConn);
-        _master->setReplSetClientCallback(this);
+        _master->setParentReplSetName(_setName);
         _master->setRunCommandHook(_runCommandHook);
         _master->setPostRunCommandHook(_postRunCommandHook);
 
@@ -748,7 +748,7 @@ namespace {
                 newConn != NULL);
 
         _lastSlaveOkConn.reset(newConn);
-        _lastSlaveOkConn->setReplSetClientCallback(this);
+        _lastSlaveOkConn->setParentReplSetName(_setName);
         _lastSlaveOkConn->setRunCommandHook(_runCommandHook);
         _lastSlaveOkConn->setPostRunCommandHook(_postRunCommandHook);
 
diff --git a/src/mongo/client/dbclientinterface.h b/src/mongo/client/dbclientinterface.h
index 05f62affef8..82927fc7ee6 100644
--- a/src/mongo/client/dbclientinterface.h
+++ b/src/mongo/client/dbclientinterface.h
@@ -1098,14 +1098,10 @@ namespace mongo {
 
         /**
            @param _autoReconnect if true, automatically reconnect on a connection failure
-           @param cp used by DBClientReplicaSet.  You do not need to specify this parameter
            @param timeout tcp timeout in seconds - this is for read/write, not connect.
            Connect timeout is fixed, but short, at 5 seconds.
          */
-        DBClientConnection(bool _autoReconnect=false, DBClientReplicaSet* cp=0, double so_timeout=0) :
-            clientSet(cp), _failed(false), autoReconnect(_autoReconnect), autoReconnectBackoff(1000, 2000), _so_timeout(so_timeout) {
-            _numConnections.fetchAndAdd(1);
-        }
+        DBClientConnection(bool _autoReconnect = false, double so_timeout = 0);
 
         virtual ~DBClientConnection() {
             _numConnections.fetchAndAdd(-1);
@@ -1200,16 +1196,10 @@ namespace mongo {
         }
 
         /**
-         * Primarily used for notifying the replica set client that the server
-         * it is talking to is not primary anymore.
-         *
-         * @param rsClient caller is responsible for managing the life of rsClient
-         * and making sure that it lives longer than this object.
-         *
-         * Warning: This is only for internal use and will eventually be removed in
-         * the future.
+         * Set the name of the replica set that this connection is associated to.
+         * Note: There is no validation on replSetName.
          */
-        void setReplSetClientCallback(DBClientReplicaSet* rsClient);
+        void setParentReplSetName(const std::string& replSetName);
 
         static void setLazyKillCursor( bool lazy ) { _lazyKillCursor = lazy; }
         static bool getLazyKillCursor() { return _lazyKillCursor; }
@@ -1221,7 +1211,6 @@ namespace mongo {
         virtual void _auth(const BSONObj& params);
         virtual void sayPiggyBack( Message &toSend );
 
-        DBClientReplicaSet *clientSet;
         boost::scoped_ptr<MessagingPort> p;
         boost::scoped_ptr<SockAddr> server;
         bool _failed;
@@ -1245,6 +1234,19 @@ namespace mongo {
 #ifdef MONGO_CONFIG_SSL
         SSLManagerInterface* sslManager();
 #endif
+
+    private:
+
+        /**
+         * Checks the BSONElement for the 'not master' keyword and if it does exist,
+         * try to inform the replica set monitor that the host this connects to is
+         * no longer primary.
+         */
+        void handleNotMasterResponse(const BSONElement& elemToCheck);
+
+        // Contains the string for the replica set name of the host this is connected to.
+        // Should be empty if this connection is not pointing to a replica set member.
+        std::string _parentReplSetName;
     };
 
     /** pings server to check if it's up
diff --git a/src/mongo/db/repl/oplogreader.cpp b/src/mongo/db/repl/oplogreader.cpp
index 0301cbb0311..5a0e1054d12 100644
--- a/src/mongo/db/repl/oplogreader.cpp
+++ b/src/mongo/db/repl/oplogreader.cpp
@@ -91,7 +91,6 @@ namespace repl {
         if (conn() == NULL || _host != host) {
             resetConnection();
             _conn = shared_ptr<DBClientConnection>(new DBClientConnection(false,
-                                                                          0,
                                                                           tcp_timeout));
             string errmsg;
             if ( !_conn->connect(host, errmsg) ||
diff --git a/src/mongo/db/repl/sync_source_feedback.cpp b/src/mongo/db/repl/sync_source_feedback.cpp
index ae909cfd95b..674555f7c53 100644
--- a/src/mongo/db/repl/sync_source_feedback.cpp
+++ b/src/mongo/db/repl/sync_source_feedback.cpp
@@ -79,7 +79,7 @@ namespace repl {
             return true;
         }
         log() << "setting syncSourceFeedback to " << host.toString();
-        _connection.reset(new DBClientConnection(false, 0, OplogReader::tcp_timeout));
+        _connection.reset(new DBClientConnection(false, OplogReader::tcp_timeout));
         string errmsg;
         try {
             if (!_connection->connect(host, errmsg) ||
diff --git a/src/mongo/dbtests/perftests.cpp b/src/mongo/dbtests/perftests.cpp
index 533019f440d..44758f346ae 100644
--- a/src/mongo/dbtests/perftests.cpp
+++ b/src/mongo/dbtests/perftests.cpp
@@ -154,7 +154,7 @@ namespace PerfTests {
                     }
                 }
 
-                boost::shared_ptr<DBClientConnection> c(new DBClientConnection(false, 0, 60));
+                boost::shared_ptr<DBClientConnection> c(new DBClientConnection(false, 60));
                 string err;
                 if( c->connect(HostAndPort("perfdb.10gen.cc"), err) ) {
                     if( !c->auth("perf", "perf", pwd, err) ) {