SERVER-32198 Make ShardedConnectionInfo::getVersion return boost::optional

This allows the logic for making the shard version decision to be
concentrated in CollectionShardingState entirely.

6 files changed, 39 insertions, 47 deletions

diff --git a/jstests/sharding/multi_mongos2.js b/jstests/sharding/multi_mongos2.js
index aef63e6b684..27d6dd447c7 100644
--- a/jstests/sharding/multi_mongos2.js
+++ b/jstests/sharding/multi_mongos2.js
@@ -6,26 +6,20 @@
 
     assert.commandWorked(st.s0.adminCommand({enablesharding: "test"}));
     st.ensurePrimaryShard('test', st.shard1.shardName);
+
+    // "test.foo" - sharded (by mongos 0)
     assert.commandWorked(st.s0.adminCommand({shardcollection: "test.foo", key: {num: 1}}));
 
-    // Test queries
+    // "test.existing" - unsharded
     assert.writeOK(st.s0.getDB('test').existing.insert({_id: 1}));
     assert.eq(1, st.s0.getDB('test').existing.count({_id: 1}));
     assert.eq(1, st.s1.getDB('test').existing.count({_id: 1}));
 
+    // "test.existing" - unsharded to sharded (by mongos 1)
     assert.commandWorked(st.s1.adminCommand({shardcollection: "test.existing", key: {_id: 1}}));
     assert.commandWorked(st.s1.adminCommand({split: "test.existing", middle: {_id: 5}}));
-
-    assert.commandWorked(st.s1.adminCommand({
-        moveChunk: "test.existing",
-        find: {_id: 1},
-        to: st.getOther(st.getPrimaryShard("test")).name
-    }));
-
-    printjson(st.s1.adminCommand({"getShardVersion": "test.existing"}));
-    printjson(new Mongo(st.getPrimaryShard("test").name).getDB("admin").adminCommand({
-        "getShardVersion": "test.existing"
-    }));
+    assert.commandWorked(
+        st.s1.adminCommand({moveChunk: "test.existing", find: {_id: 1}, to: st.shard0.shardName}));
 
     assert.eq(1, st.s0.getDB('test').existing.count({_id: 1}));  // SERVER-2828
     assert.eq(1, st.s1.getDB('test').existing.count({_id: 1}));
@@ -52,12 +46,8 @@
 
     assert.writeOK(st.s1.adminCommand({shardcollection: "test.existing3", key: {_id: 1}}));
     assert.commandWorked(st.s1.adminCommand({split: "test.existing3", middle: {_id: 5}}));
-
-    assert.commandWorked(st.s0.adminCommand({
-        moveChunk: "test.existing3",
-        find: {_id: 1},
-        to: st.getOther(st.getPrimaryShard("test")).name
-    }));
+    assert.commandWorked(
+        st.s0.adminCommand({moveChunk: "test.existing3", find: {_id: 1}, to: st.shard0.shardName}));
 
     st.stop();
 })();
diff --git a/src/mongo/db/s/collection_sharding_state.cpp b/src/mongo/db/s/collection_sharding_state.cpp
index d19955f6a3c..a6a6765d754 100644
--- a/src/mongo/db/s/collection_sharding_state.cpp
+++ b/src/mongo/db/s/collection_sharding_state.cpp
@@ -366,7 +366,6 @@ bool CollectionShardingState::_checkShardVersionOk(OperationContext* opCtx,
                                                    ChunkVersion* expectedShardVersion,
                                                    ChunkVersion* actualShardVersion) {
     auto* const client = opCtx->getClient();
-
     auto& oss = OperationShardingState::get(opCtx);
 
     // If there is a version attached to the OperationContext, use it as the received version.
@@ -382,7 +381,12 @@ bool CollectionShardingState::_checkShardVersionOk(OperationContext* opCtx,
             return true;
         }
 
-        *expectedShardVersion = info->getVersion(_nss.ns());
+        auto connectionExpectedShardVersion = info->getVersion(_nss.ns());
+        if (!connectionExpectedShardVersion) {
+            *expectedShardVersion = ChunkVersion::UNSHARDED();
+        } else {
+            *expectedShardVersion = std::move(*connectionExpectedShardVersion);
+        }
     }
 
     // An operation with read concern 'available' should never have shardVersion set.
diff --git a/src/mongo/db/s/get_shard_version_command.cpp b/src/mongo/db/s/get_shard_version_command.cpp
index 17a2d4d6318..c722ae3eb65 100644
--- a/src/mongo/db/s/get_shard_version_command.cpp
+++ b/src/mongo/db/s/get_shard_version_command.cpp
@@ -99,8 +99,9 @@ public:
 
         ShardedConnectionInfo* const sci = ShardedConnectionInfo::get(opCtx->getClient(), false);
         result.appendBool("inShardedMode", sci != nullptr);
-        if (sci) {
-            result.appendTimestamp("mine", sci->getVersion(nss.ns()).toLong());
+
+        if (sci && sci->getVersion(nss.ns())) {
+            result.appendTimestamp("mine", sci->getVersion(nss.ns())->toLong());
         } else {
             result.appendTimestamp("mine", 0);
         }
diff --git a/src/mongo/db/s/set_shard_version_command.cpp b/src/mongo/db/s/set_shard_version_command.cpp
index 73227b9e1fb..076c00a820d 100644
--- a/src/mongo/db/s/set_shard_version_command.cpp
+++ b/src/mongo/db/s/set_shard_version_command.cpp
@@ -196,7 +196,14 @@ public:
 
         // Step 4
 
-        const ChunkVersion connectionVersion = info->getVersion(nss.ns());
+        const auto connectionVersionOrNotSet = info->getVersion(nss.ns());
+
+        // For backwards compatibility, calling SSV for a namespace which is sharded, but doesn't
+        // have version set on the connection requires the call to fail and require the
+        // "need_authoritative" flag to be set on the response. Treating unset connection versions
+        // as UNSHARDED is the legacy way to achieve this purpose.
+        const auto connectionVersion =
+            (connectionVersionOrNotSet ? *connectionVersionOrNotSet : ChunkVersion::UNSHARDED());
         connectionVersion.addToBSON(result, "oldVersion");
 
         {
@@ -226,19 +233,19 @@ public:
                                     : ChunkVersion::UNSHARDED());
 
             if (requestedVersion.isWriteCompatibleWith(collectionShardVersion)) {
-                // mongos and mongod agree!
+                // MongoS and MongoD agree on what is the collection's shard version
+                //
                 // Now we should update the connection's version if it's not compatible with the
                 // request's version. This could happen if the shard's metadata has changed, but
                 // the remote client has already refreshed its view of the metadata since the last
                 // time it sent anything over this connection.
                 if (!connectionVersion.isWriteCompatibleWith(requestedVersion)) {
-                    // A migration occurred.
                     if (connectionVersion < collectionShardVersion &&
                         connectionVersion.epoch() == collectionShardVersion.epoch()) {
+                        // A migration occurred
                         info->setVersion(nss.ns(), requestedVersion);
-                    }
-                    // The collection was dropped and recreated.
-                    else if (authoritative) {
+                    } else if (authoritative) {
+                        // The collection was dropped and recreated
                         info->setVersion(nss.ns(), requestedVersion);
                     } else {
                         result.append("ns", nss.ns());
diff --git a/src/mongo/db/s/sharded_connection_info.cpp b/src/mongo/db/s/sharded_connection_info.cpp
index d9154ac7f42..338fcf3bd5e 100644
--- a/src/mongo/db/s/sharded_connection_info.cpp
+++ b/src/mongo/db/s/sharded_connection_info.cpp
@@ -32,18 +32,12 @@
 
 #include "mongo/db/s/sharded_connection_info.h"
 
-#include <boost/optional.hpp>
-#include <boost/utility/in_place_factory.hpp>
-
 #include "mongo/client/global_conn_pool.h"
 #include "mongo/db/client.h"
 #include "mongo/db/logical_time_metadata_hook.h"
-#include "mongo/db/operation_context.h"
 #include "mongo/db/s/sharding_egress_metadata_hook_for_mongod.h"
-#include "mongo/db/service_context.h"
 #include "mongo/platform/atomic_word.h"
 #include "mongo/rpc/metadata/egress_metadata_hook_list.h"
-#include "mongo/s/chunk_version.h"
 #include "mongo/s/client/shard_connection.h"
 #include "mongo/s/client/sharding_connection_hook.h"
 #include "mongo/stdx/memory.h"
@@ -60,9 +54,7 @@ AtomicUInt32 alreadyAddedHook{0};
 
 }  // namespace
 
-ShardedConnectionInfo::ShardedConnectionInfo() {
-    _forceVersionOk = false;
-}
+ShardedConnectionInfo::ShardedConnectionInfo() = default;
 
 ShardedConnectionInfo::~ShardedConnectionInfo() = default;
 
@@ -71,7 +63,7 @@ ShardedConnectionInfo* ShardedConnectionInfo::get(Client* client, bool create) {
 
     if (!current && create) {
         LOG(1) << "entering shard mode for connection";
-        current = boost::in_place();
+        current.emplace();
     }
 
     return current ? &current.value() : nullptr;
@@ -81,12 +73,12 @@ void ShardedConnectionInfo::reset(Client* client) {
     clientSCI(client) = boost::none;
 }
 
-ChunkVersion ShardedConnectionInfo::getVersion(const std::string& ns) const {
+boost::optional<ChunkVersion> ShardedConnectionInfo::getVersion(const std::string& ns) const {
     NSVersionMap::const_iterator it = _versions.find(ns);
     if (it != _versions.end()) {
         return it->second;
     } else {
-        return ChunkVersion::UNSHARDED();
+        return boost::none;
     }
 }
 
diff --git a/src/mongo/db/s/sharded_connection_info.h b/src/mongo/db/s/sharded_connection_info.h
index 9efc734fe2c..189fb0fe20d 100644
--- a/src/mongo/db/s/sharded_connection_info.h
+++ b/src/mongo/db/s/sharded_connection_info.h
@@ -28,14 +28,15 @@
 
 #pragma once
 
+#include <boost/optional.hpp>
 #include <map>
 #include <string>
 
 #include "mongo/base/disallow_copying.h"
+#include "mongo/s/chunk_version.h"
 
 namespace mongo {
 
-struct ChunkVersion;
 class Client;
 class ServiceContext;
 
@@ -51,19 +52,19 @@ public:
     ~ShardedConnectionInfo();
 
     static ShardedConnectionInfo* get(Client* client, bool create);
+    static void reset(Client* client);
 
     /**
      * Returns the shard version associated with the specified namespace on this connection. If no
-     * version is associated with the namespace returns ChunkVersion::UNSHARDED.
+     * version is associated with the namespace returns boost::none.
      */
-    ChunkVersion getVersion(const std::string& ns) const;
+    boost::optional<ChunkVersion> getVersion(const std::string& ns) const;
 
     /**
      * Assigns a new version on the connection to the specified namespace.
      */
     void setVersion(const std::string& ns, const ChunkVersion& version);
 
-    static void reset(Client* client);
     static void addHook(ServiceContext* service);
 
 private:
@@ -72,9 +73,6 @@ private:
     // Map from a namespace string to the chunk version with which this connection has been
     // initialized for the specified namespace
     NSVersionMap _versions;
-
-    // If this is true, then chunk versions aren't checked, and all operations are allowed
-    bool _forceVersionOk;
 };