SERVER-14616 replace BSONObj with HandshakeArgs and eliminate redundant handshake method

15 files changed, 287 insertions, 165 deletions

diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript
index b4f855e923b..ecc9522a3fd 100644
--- a/src/mongo/db/repl/SConscript
+++ b/src/mongo/db/repl/SConscript
@@ -82,6 +82,7 @@ env.Library('replmocks',
 
 env.Library('replica_set_messages',
             [
+                'handshake_args.cpp',
                 'member_config.cpp',
                 'repl_set_heartbeat_args.cpp',
                 'repl_set_heartbeat_response.cpp',
diff --git a/src/mongo/db/repl/handshake_args.cpp b/src/mongo/db/repl/handshake_args.cpp
new file mode 100644
index 00000000000..9a81c4ddadf
--- /dev/null
+++ b/src/mongo/db/repl/handshake_args.cpp
@@ -0,0 +1,114 @@
+/**
+ *    Copyright 2014 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.
+ */
+
+#include "mongo/platform/basic.h"
+
+#include "mongo/db/repl/handshake_args.h"
+
+#include "mongo/base/status.h"
+#include "mongo/bson/util/bson_check.h"
+#include "mongo/bson/util/bson_extract.h"
+#include "mongo/db/jsobj.h"
+
+namespace mongo {
+namespace repl {
+
+namespace {
+
+    const std::string kRIDFieldName = "handshake";
+    // TODO(danneberg) remove after 2.8 since this field is only allowed for backwards compatibility
+    const std::string kOldMemberConfigFieldName = "config";
+    const std::string kMemberIdFieldName = "member";
+
+    const std::string kLegalHandshakeFieldNames[] = {
+        kRIDFieldName,
+        kOldMemberConfigFieldName,
+        kMemberIdFieldName
+    };
+
+} // namespace
+
+    HandshakeArgs::HandshakeArgs() :
+        _hasRid(false),
+        _hasMemberId(false),
+        _rid(OID()),
+        _memberId(-1) {}
+
+    Status HandshakeArgs::initialize(const BSONObj& argsObj) {
+        Status status = bsonCheckOnlyHasFields("HandshakeArgs",
+                                               argsObj,
+                                               kLegalHandshakeFieldNames);
+        if (!status.isOK())
+            return status;
+
+        BSONElement oid;
+        status = bsonExtractTypedField(argsObj, kRIDFieldName, jstOID, &oid);
+        if (!status.isOK())
+            return status;
+        _rid = oid.OID();
+        _hasRid = true;
+        
+        status = bsonExtractIntegerField(argsObj, kMemberIdFieldName, &_memberId);
+        if (!status.isOK()) {
+            // field not necessary for master slave, do not return NoSuchKey Error
+            if (status != ErrorCodes::NoSuchKey) {
+                return status;
+            }
+            _memberId = -1;
+        }
+        else {
+            _hasMemberId = true;
+        }
+
+        return Status::OK();
+    }
+
+    bool HandshakeArgs::isInitialized() const {
+        return _hasRid;
+    }
+
+    void HandshakeArgs::setRid(const OID& newVal) {
+        _rid = newVal;
+        _hasRid = true;
+    }
+
+    void HandshakeArgs::setMemberId(long long newVal) {
+        _memberId = newVal;
+        _hasMemberId = true;
+    }
+
+    BSONObj HandshakeArgs::toBSON() const {
+        invariant(isInitialized());
+        BSONObjBuilder builder;
+        builder.append(kRIDFieldName, _rid);
+        builder.append(kMemberIdFieldName, _memberId);
+        return builder.obj();
+    }
+
+}  // namespace repl
+}  // namespace mongo
diff --git a/src/mongo/db/repl/handshake_args.h b/src/mongo/db/repl/handshake_args.h
new file mode 100644
index 00000000000..b0d442aaaf6
--- /dev/null
+++ b/src/mongo/db/repl/handshake_args.h
@@ -0,0 +1,95 @@
+/**
+ *    Copyright (C) 2014 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 "mongo/db/jsobj.h"
+
+namespace mongo {
+
+    class Status;
+
+namespace repl {
+
+    /**
+     * Arguments to the handshake command.
+     */
+    class HandshakeArgs {
+    public:
+        HandshakeArgs();
+
+        /**
+         * Initializes this HandshakeArgs from the contents of args.
+         */
+        Status initialize(const BSONObj& argsObj);
+
+        /**
+         * Returns true if all required fields have been initialized.
+         */
+        bool isInitialized() const;
+
+        /**
+         * Gets the _id of the sender in their ReplSetConfig.
+         */
+        long long getMemberId() const { return _memberId; }
+
+        /**
+         * Gets the unique identifier of the sender, which is used to track replication progress.
+         */
+        OID getRid() const { return _rid; }
+
+        /**
+         * The below methods check whether or not value in the method name has been set.
+         */
+        bool hasRid() { return _hasRid; };
+        bool hasMemberId() { return _hasMemberId; };
+
+        /**
+         * The below methods set the value in the method name to 'newVal'.
+         */
+        void setRid(const OID& newVal);
+        void setMemberId(long long newVal);
+
+        /**
+         * Returns a BSONified version of the object.
+         * Should only be called if the mandatory fields have been set.
+         * Optional fields are only included if they have been set.
+         */
+        BSONObj toBSON() const;
+
+    private:
+        bool _hasRid;
+        bool _hasMemberId;
+
+        // look at the body of the isInitialized() function to see which fields are mandatory
+        OID _rid;
+        long long _memberId;
+    };
+
+} // namespace repl
+} // namespace mongo
diff --git a/src/mongo/db/repl/repl_coordinator.h b/src/mongo/db/repl/repl_coordinator.h
index 9f56b7233df..a3b509cb737 100644
--- a/src/mongo/db/repl/repl_coordinator.h
+++ b/src/mongo/db/repl/repl_coordinator.h
@@ -34,6 +34,7 @@
 #include "mongo/base/disallow_copying.h"
 #include "mongo/base/status.h"
 #include "mongo/db/jsobj.h"
+#include "mongo/db/repl/handshake_args.h"
 #include "mongo/db/repl/member_state.h"
 #include "mongo/db/repl/replication_executor.h"
 #include "mongo/db/repl/repl_settings.h"
@@ -411,29 +412,15 @@ namespace repl {
                                                     BSONObjBuilder* resultObj) = 0;
 
         /**
-         * Handles an incoming replSetUpdatePosition command that contains a handshake.
-         * returns the same codes as processHandshake below, as well as any of the normal replset
-         * command ErrorCodes.
-         * TODO(spencer): Remove this method in favor of just using processHandshake
-         */
-        virtual Status processReplSetUpdatePositionHandshake(const OperationContext* txn,
-                                                             const BSONObj& handshake,
-                                                             BSONObjBuilder* resultObj) = 0;
-
-        /**
          * Handles an incoming Handshake command (or a handshake from replSetUpdatePosition).
          * Associates the node's 'remoteID' with its 'handshake' object. This association is used
          * to update local.slaves and to forward the node's replication progress upstream when this
          * node is being chained through.
          *
-         * Returns ErrorCodes::ProtocolError if the handshake is missing required fields and
-         * ErrorCodes::NodeNotFound if no replica set member is found with the given member ID.
-         *
-         * TODO(spencer): Remove remoteID arg and get it from the handshake instead.
+         * Returns ErrorCodes::NodeNotFound if no replica set member exists with the given member ID
          */
         virtual Status processHandshake(const OperationContext* txn,
-                                        const OID& remoteID,
-                                        const BSONObj& handshake) = 0;
+                                        const HandshakeArgs& handshake) = 0;
 
         /**
          * Returns once the oplog's most recent entry changes or after one second, whichever
diff --git a/src/mongo/db/repl/repl_coordinator_hybrid.cpp b/src/mongo/db/repl/repl_coordinator_hybrid.cpp
index 031cda0bd08..d1eccfe8b63 100644
--- a/src/mongo/db/repl/repl_coordinator_hybrid.cpp
+++ b/src/mongo/db/repl/repl_coordinator_hybrid.cpp
@@ -287,27 +287,11 @@ namespace repl {
         return legacyStatus;
     }
 
-    Status HybridReplicationCoordinator::processReplSetUpdatePositionHandshake(
-            const OperationContext* txn,
-            const BSONObj& handshake,
-            BSONObjBuilder* resultObj) {
-        Status legacyStatus = _legacy.processReplSetUpdatePositionHandshake(txn,
-                                                                            handshake,
-                                                                            resultObj);
-        // TODO(spencer): Can't call into the impl until it can load a valid config
-        //BSONObjBuilder implResult;
-        //Status implStatus = _impl.processReplSetUpdatePositionHandshake(txn,
-        //                                                                handshake,
-        //                                                                &implResult);
-        return legacyStatus;
-    }
-
     Status HybridReplicationCoordinator::processHandshake(const OperationContext* txn,
-                                                          const OID& remoteID,
-                                                          const BSONObj& handshake) {
-        Status legacyResponse = _legacy.processHandshake(txn, remoteID, handshake);
+                                                          const HandshakeArgs& handshake) {
+        Status legacyResponse = _legacy.processHandshake(txn, handshake);
         // TODO(spencer): Can't call into the impl until it can load a valid config
-        //_impl.processHandshake(txn, remoteID, handshake);
+        //_impl.processHandshake(txn, handshake);
         return legacyResponse;
     }
 
diff --git a/src/mongo/db/repl/repl_coordinator_hybrid.h b/src/mongo/db/repl/repl_coordinator_hybrid.h
index 8ccb7e6f01c..af3bd912ee9 100644
--- a/src/mongo/db/repl/repl_coordinator_hybrid.h
+++ b/src/mongo/db/repl/repl_coordinator_hybrid.h
@@ -145,13 +145,8 @@ namespace repl {
                                                     const BSONArray& updates,
                                                     BSONObjBuilder* resultObj);
 
-        virtual Status processReplSetUpdatePositionHandshake(const OperationContext* txn,
-                                                             const BSONObj& handshake,
-                                                             BSONObjBuilder* resultObj);
-
         virtual Status processHandshake(const OperationContext* txn,
-                                        const OID& remoteID,
-                                        const BSONObj& handshake);
+                                        const HandshakeArgs& handshake);
 
         virtual void waitUpToOneSecondForOptimeChange(const OpTime& ot);
 
diff --git a/src/mongo/db/repl/repl_coordinator_impl.cpp b/src/mongo/db/repl/repl_coordinator_impl.cpp
index 238f6a68611..097da80c257 100644
--- a/src/mongo/db/repl/repl_coordinator_impl.cpp
+++ b/src/mongo/db/repl/repl_coordinator_impl.cpp
@@ -655,33 +655,14 @@ namespace repl {
         return Status::OK();
     }
 
-    Status ReplicationCoordinatorImpl::processReplSetUpdatePositionHandshake(
-            const OperationContext* txn,
-            const BSONObj& cmdObj,
-            BSONObjBuilder* resultObj) {
-        OID rid = cmdObj["handshake"].OID();
-        Status status = processHandshake(txn, rid, cmdObj);
-        if (!status.isOK()) {
-            return status;
-        }
-
-        return Status::OK();
-    }
-
     Status ReplicationCoordinatorImpl::processHandshake(const OperationContext* txn,
-                                                        const OID& remoteID,
-                                                        const BSONObj& handshake) {
-        LOG(2) << "Received handshake " << handshake << " from node with RID " << remoteID;
+                                                        const HandshakeArgs& handshake) {
+        LOG(2) << "Received handshake " << handshake.toBSON();
 
         boost::lock_guard<boost::mutex> lock(_mutex);
-        SlaveInfo& slaveInfo = _slaveInfoMap[remoteID];
+        SlaveInfo& slaveInfo = _slaveInfoMap[handshake.getRid()];
         if (_getReplicationMode_inlock() == modeReplSet) {
-            if (!handshake.hasField("member")) {
-                return Status(ErrorCodes::ProtocolError,
-                              str::stream() << "Handshake object did not contain \"member\" field. "
-                                      "Handshake: " << handshake);
-            }
-            int memberID = handshake["member"].Int();
+            int memberID = handshake.getMemberId();
             const MemberConfig* member = _rsConfig.findMemberByID(memberID);
             if (!member) {
                 return Status(ErrorCodes::NodeNotFound,
diff --git a/src/mongo/db/repl/repl_coordinator_impl.h b/src/mongo/db/repl/repl_coordinator_impl.h
index f7aeaad9fa0..54511243db8 100644
--- a/src/mongo/db/repl/repl_coordinator_impl.h
+++ b/src/mongo/db/repl/repl_coordinator_impl.h
@@ -157,13 +157,8 @@ namespace repl {
                                                     const BSONArray& updates,
                                                     BSONObjBuilder* resultObj);
 
-        virtual Status processReplSetUpdatePositionHandshake(const OperationContext* txn,
-                                                             const BSONObj& handshake,
-                                                             BSONObjBuilder* resultObj);
-
         virtual Status processHandshake(const OperationContext* txn,
-                                        const OID& remoteID,
-                                        const BSONObj& handshake);
+                                        const HandshakeArgs& handshake);
 
         virtual void waitUpToOneSecondForOptimeChange(const OpTime& ot);
 
diff --git a/src/mongo/db/repl/repl_coordinator_impl_test.cpp b/src/mongo/db/repl/repl_coordinator_impl_test.cpp
index 7f65ac95307..654a57b4fd6 100644
--- a/src/mongo/db/repl/repl_coordinator_impl_test.cpp
+++ b/src/mongo/db/repl/repl_coordinator_impl_test.cpp
@@ -430,19 +430,22 @@ namespace {
         OID rid1 = OID::gen();
         OID rid2 = OID::gen();
         OID rid3 = OID::gen();
-        BSONObj handshake1 = BSON("handshake" << rid1 <<
-                                  "member" << 0 <<
-                                  "config" << BSON("_id" << 0 << "host" << "test1:1234"));
-        BSONObj handshake2 = BSON("handshake" << rid2 <<
-                                  "member" << 1 <<
-                                  "config" << BSON("_id" << 1 << "host" << "test2:1234"));
-        BSONObj handshake3 = BSON("handshake" << rid3 <<
-                                  "member" << 2 <<
-                                  "config" << BSON("_id" << 2 << "host" << "test3:1234"));
+        HandshakeArgs handshake1;
+        handshake1.initialize(BSON("handshake" << rid1 <<
+                                   "member" << 0 <<
+                                   "config" << BSON("_id" << 0 << "host" << "test1:1234")));
+        HandshakeArgs handshake2;
+        handshake2.initialize(BSON("handshake" << rid2 <<
+                                   "member" << 1 <<
+                                   "config" << BSON("_id" << 1 << "host" << "test2:1234")));
+        HandshakeArgs handshake3;
+        handshake3.initialize(BSON("handshake" << rid3 <<
+                                   "member" << 2 <<
+                                   "config" << BSON("_id" << 2 << "host" << "test3:1234")));
         OperationContextNoop txn;
-        ASSERT_OK(coordinator->processHandshake(&txn, rid1, handshake1));
-        ASSERT_OK(coordinator->processHandshake(&txn, rid2, handshake2));
-        ASSERT_OK(coordinator->processHandshake(&txn, rid3, handshake3));
+        ASSERT_OK(coordinator->processHandshake(&txn, handshake1));
+        ASSERT_OK(coordinator->processHandshake(&txn, handshake2));
+        ASSERT_OK(coordinator->processHandshake(&txn, handshake3));
         OpTime optime1(1, 1);
         OpTime optime2(1, 2);
         OpTime optime3(2, 1);
@@ -507,14 +510,16 @@ namespace {
         // Have other nodes handshake us and make sure we process it right.
         OID slave1RID = OID::gen();
         OID slave2RID = OID::gen();
-        BSONObj slave1Handshake = BSON("handshake" << slave1RID <<
-                                       "member" << 0 <<
-                                       "config" << BSON("_id" << 0 << "host" << "test1:1234"));
-        BSONObj slave2Handshake = BSON("handshake" << slave2RID <<
-                                       "member" << 2 <<
-                                       "config" << BSON("_id" << 2 << "host" << "test2:1234"));
-        ASSERT_OK(coordinator->processHandshake(&txn, slave1RID, slave1Handshake));
-        ASSERT_OK(coordinator->processHandshake(&txn, slave2RID, slave2Handshake));
+        HandshakeArgs slave1Handshake;
+        slave1Handshake.initialize(BSON("handshake" << slave1RID <<
+                                        "member" << 0 <<
+                                        "config" << BSON("_id" << 0 << "host" << "test1:1234")));
+        HandshakeArgs slave2Handshake;
+        slave2Handshake.initialize(BSON("handshake" << slave2RID <<
+                                        "member" << 2 <<
+                                        "config" << BSON("_id" << 2 << "host" << "test2:1234")));
+        ASSERT_OK(coordinator->processHandshake(&txn, slave1Handshake));
+        ASSERT_OK(coordinator->processHandshake(&txn, slave2Handshake));
 
         coordinator->prepareReplSetUpdatePositionCommandHandshakes(&txn, &handshakes);
         ASSERT_EQUALS(3U, handshakes.size());
diff --git a/src/mongo/db/repl/repl_coordinator_legacy.cpp b/src/mongo/db/repl/repl_coordinator_legacy.cpp
index 69b56ff528b..9c1225dd2b6 100644
--- a/src/mongo/db/repl/repl_coordinator_legacy.cpp
+++ b/src/mongo/db/repl/repl_coordinator_legacy.cpp
@@ -47,6 +47,7 @@
 #include "mongo/db/repl/repl_settings.h"
 #include "mongo/db/repl/replset_commands.h"
 #include "mongo/db/repl/rs.h"
+#include "mongo/db/repl/rs_config.h"
 #include "mongo/db/repl/rs_initiate.h"
 #include "mongo/db/repl/write_concern.h"
 #include "mongo/db/write_concern_options.h"
@@ -382,13 +383,17 @@ namespace {
                 // Only update if ts is newer than what we have already
                 return Status::OK();
             }
-            BSONObj config = mapFindWithDefault(_ridConfigMap, rid, BSONObj());
-            LOG(2) << "received notification that node with RID " << rid << " and config " << config
-                    << " has reached optime: " << ts.toStringPretty();
 
             if (rid != getMyRID(txn)) {
-                // TODO(spencer): Remove this invariant for backwards compatibility
-                invariant(!config.isEmpty());
+                BSONObj config;
+                if (getReplicationMode() == modeReplSet) {
+                    Member* mem = _ridMemberMap[rid];
+                    invariant(mem);
+                    config = BSON("_id" << mem->id());
+                }
+                LOG(2) << "received notification that node with RID " << rid << " and config "
+                       << config << " has reached optime: " << ts.toStringPretty();
+
                 // This is what updates the progress information used for satisfying write concern
                 // and wakes up threads waiting for replication.
                 if (!updateSlaveTracking(BSON("_id" << rid), config, ts)) {
@@ -442,7 +447,6 @@ namespace {
             for (SlaveOpTimeMap::const_iterator itr = _slaveOpTimeMap.begin();
                     itr != _slaveOpTimeMap.end(); ++itr) {
                 const OID& rid = itr->first;
-                const BSONObj& config = mapFindWithDefault(_ridConfigMap, rid, BSONObj());
                 BSONObjBuilder entry(arrayBuilder.subobjStart());
                 entry.append("_id", rid);
                 entry.append("optime", itr->second);
@@ -453,6 +457,9 @@ namespace {
                     entry.append("config", theReplSet->myConfig().asBson());
                 }
                 else {
+                    Member* member = _ridMemberMap[rid];
+                    invariant(member);
+                    BSONObj config = member->config().asBson();
                     entry.append("config", config);
                 }
             }
@@ -958,48 +965,16 @@ namespace {
         return Status::OK();
     }
 
-    Status LegacyReplicationCoordinator::processReplSetUpdatePositionHandshake(
-            const OperationContext* txn, const BSONObj& cmdObj, BSONObjBuilder* resultObj) {
-
-        OID rid = cmdObj["handshake"].OID();
-        Status status = processHandshake(txn, rid, cmdObj);
-        if (!status.isOK()) {
-            return status;
-        }
-
-        // if we're a replset and aren't primary, pass the handshake along
-        if (theReplSet && !theReplSet->isPrimary()) {
-            theReplSet->syncSourceFeedback.forwardSlaveHandshake();
-        }
-        return Status::OK();
-    }
-
     Status LegacyReplicationCoordinator::processHandshake(const OperationContext* txn,
-                                                          const OID& remoteID,
-                                                          const BSONObj& handshake) {
-        LOG(2) << "Received handshake " << handshake << " from node with RID " << remoteID;
+                                                          const HandshakeArgs& handshake) {
+        LOG(2) << "Received handshake " << handshake.toBSON();
 
         boost::lock_guard<boost::mutex> lock(_mutex);
-        BSONObj configObj;
-        if (handshake.hasField("config")) {
-            configObj = handshake["config"].Obj().getOwned();
-        } else {
-            configObj = BSON("host" << txn->getClient()->clientAddress(true) <<
-                             "upgradeNeeded" << true);
-        }
-        _ridConfigMap[remoteID] = configObj;
-
         if (getReplicationMode() != modeReplSet) {
             return Status::OK();
         }
 
-        if (!handshake.hasField("member")) {
-            return Status(ErrorCodes::ProtocolError,
-                          str::stream() << "Handshake object did not contain \"member\" field.  "
-                                  "Handshake" << handshake);
-        }
-
-        int memberID = handshake["member"].Int();
+        int memberID = handshake.getMemberId();
         Member* member = theReplSet->getMutableMember(memberID);
         // it is possible that a node that was removed in a reconfig tried to handshake this node
         // in that case, the Member will no longer be in theReplSet's _members List and member
@@ -1010,7 +985,7 @@ namespace {
                                   " could not be found in replica set config during handshake");
         }
 
-        _ridMemberMap[remoteID] = member;
+        _ridMemberMap[handshake.getRid()] = member;
         theReplSet->syncSourceFeedback.forwardSlaveHandshake();
         return Status::OK();
     }
diff --git a/src/mongo/db/repl/repl_coordinator_legacy.h b/src/mongo/db/repl/repl_coordinator_legacy.h
index 9721b8be0be..0ec889e25aa 100644
--- a/src/mongo/db/repl/repl_coordinator_legacy.h
+++ b/src/mongo/db/repl/repl_coordinator_legacy.h
@@ -143,13 +143,8 @@ namespace repl {
                                                     const BSONArray& updates,
                                                     BSONObjBuilder* resultObj);
 
-        virtual Status processReplSetUpdatePositionHandshake(const OperationContext* txn,
-                                                             const BSONObj& handshake,
-                                                             BSONObjBuilder* resultObj);
-
         virtual Status processHandshake(const OperationContext* txn,
-                                        const OID& remoteID,
-                                        const BSONObj& handshake);
+                                        const HandshakeArgs& handshake);
 
         virtual void waitUpToOneSecondForOptimeChange(const OpTime& ot);
 
@@ -170,12 +165,9 @@ namespace repl {
                                const Milliseconds& stepdownTime,
                                const Milliseconds& postStepdownWaitTime);
 
-        // Mutex that protects the _ridConfigMap and the _slaveOpTimeMap;
+        // Mutex that protects the _slaveOpTimeMap
         boost::mutex _mutex;
 
-        // Map from RID to member config object
-        std::map<OID, BSONObj> _ridConfigMap;
-
         // Map from RID to Member pointer for replica set nodes
         typedef std::map<OID, Member*> OIDMemberMap;
         OIDMemberMap _ridMemberMap;
diff --git a/src/mongo/db/repl/repl_coordinator_mock.cpp b/src/mongo/db/repl/repl_coordinator_mock.cpp
index 615944cb107..891a46609a6 100644
--- a/src/mongo/db/repl/repl_coordinator_mock.cpp
+++ b/src/mongo/db/repl/repl_coordinator_mock.cpp
@@ -215,17 +215,8 @@ namespace repl {
         return Status::OK();
     }
 
-    Status ReplicationCoordinatorMock::processReplSetUpdatePositionHandshake(
-            const OperationContext* txn,
-            const BSONObj& handshake,
-            BSONObjBuilder* resultObj) {
-        // TODO
-        return Status::OK();
-    }
-
     Status ReplicationCoordinatorMock::processHandshake(const OperationContext* txn,
-                                                        const OID& remoteID,
-                                                        const BSONObj& handshake) {
+                                                        const HandshakeArgs& handshake) {
         return Status::OK();
     }
 
diff --git a/src/mongo/db/repl/repl_coordinator_mock.h b/src/mongo/db/repl/repl_coordinator_mock.h
index 1bc85e8fb2b..b83e630f8b3 100644
--- a/src/mongo/db/repl/repl_coordinator_mock.h
+++ b/src/mongo/db/repl/repl_coordinator_mock.h
@@ -144,13 +144,8 @@ namespace repl {
                                                     const BSONArray& updates,
                                                     BSONObjBuilder* resultObj);
 
-        virtual Status processReplSetUpdatePositionHandshake(const OperationContext* txn,
-                                                             const BSONObj& handshake,
-                                                             BSONObjBuilder* resultObj);
-
         virtual Status processHandshake(const OperationContext* txn,
-                                        const OID& remoteID,
-                                        const BSONObj& handshake);
+                                        const HandshakeArgs& handshake);
 
         virtual void waitUpToOneSecondForOptimeChange(const OpTime& ot);
 
diff --git a/src/mongo/db/repl/replset_commands.cpp b/src/mongo/db/repl/replset_commands.cpp
index 7b421ed06a0..98523e5e7ac 100644
--- a/src/mongo/db/repl/replset_commands.cpp
+++ b/src/mongo/db/repl/replset_commands.cpp
@@ -368,12 +368,21 @@ namespace repl {
                 // we have received a handshake, not an update message
                 // handshakes are done here to ensure the receiving end supports the update command
 
+                HandshakeArgs handshake;
+                status = handshake.initialize(cmdObj["handshake"].embeddedObject());
+                if (!status.isOK())
+                    return appendCommandStatus(result, status);
+
+                if (!handshake.hasMemberId()) {
+                    return appendCommandStatus(
+                            result,
+                            Status(ErrorCodes::NoSuchKey,
+                                   "replSetUpdatePosition handshake was missing 'member' field"));
+                }
+
                 return appendCommandStatus(
                         result,
-                        getGlobalReplicationCoordinator()->processReplSetUpdatePositionHandshake(
-                                txn,
-                                cmdObj["handshake"].embeddedObject(),
-                                &result));
+                        getGlobalReplicationCoordinator()->processHandshake(txn, handshake));
             }
 
             uassert(16888, "optimes field should be an array with an object for each secondary",
diff --git a/src/mongo/db/repl/sync_source_feedback.cpp b/src/mongo/db/repl/sync_source_feedback.cpp
index ceafe6b6a19..c85f7357f1b 100644
--- a/src/mongo/db/repl/sync_source_feedback.cpp
+++ b/src/mongo/db/repl/sync_source_feedback.cpp
@@ -94,11 +94,14 @@ namespace repl {
     }
 
     bool SyncSourceFeedback::replHandshake(OperationContext* txn) {
+        ReplicationCoordinator* replCoord = getGlobalReplicationCoordinator();
+        if (replCoord->getCurrentMemberState().primary()) {
+            // primary has no one to handshake to
+            return true;
+        }
         // construct a vector of handshake obj for us as well as all chained members
         std::vector<BSONObj> handshakeObjs;
-        getGlobalReplicationCoordinator()->prepareReplSetUpdatePositionCommandHandshakes(
-                txn,
-                &handshakeObjs);
+        replCoord->prepareReplSetUpdatePositionCommandHandshakes(txn, &handshakeObjs);
         LOG(1) << "handshaking upstream updater";
         for (std::vector<BSONObj>::iterator it = handshakeObjs.begin();
                 it != handshakeObjs.end();