SERVER-36987 ChunkVersion::minorVersion truncates to 16 bit

3 files changed, 29 insertions, 16 deletions

diff --git a/src/mongo/s/chunk_manager_refresh_bm.cpp b/src/mongo/s/chunk_manager_refresh_bm.cpp
index e965a15df48..86ee3e23060 100644
--- a/src/mongo/s/chunk_manager_refresh_bm.cpp
+++ b/src/mongo/s/chunk_manager_refresh_bm.cpp
@@ -55,14 +55,14 @@ ChunkRange getRangeForChunk(int i, int nChunks) {
 }
 
 template <typename ShardSelectorFn>
-auto makeChunkManagerWithShardSelector(int nShards, int nChunks, ShardSelectorFn selectShard) {
+auto makeChunkManagerWithShardSelector(int nShards, uint32_t nChunks, ShardSelectorFn selectShard) {
     const auto collEpoch = OID::gen();
     const auto collName = NamespaceString("test.foo");
     const auto shardKeyPattern = KeyPattern(BSON("_id" << 1));
 
     std::vector<ChunkType> chunks;
     chunks.reserve(nChunks);
-    for (int i = 0; i < nChunks; ++i) {
+    for (uint32_t i = 0; i < nChunks; ++i) {
         chunks.emplace_back(collName,
                             getRangeForChunk(i, nChunks),
                             ChunkVersion{i + 1, 0, collEpoch},
@@ -85,11 +85,11 @@ ShardId optimalShardSelector(int i, int nShards, int nChunks) {
     return ShardId(str::stream() << "shard" << shardNum);
 }
 
-NOINLINE_DECL auto makeChunkManagerWithPessimalBalancedDistribution(int nShards, int nChunks) {
+NOINLINE_DECL auto makeChunkManagerWithPessimalBalancedDistribution(int nShards, uint32_t nChunks) {
     return makeChunkManagerWithShardSelector(nShards, nChunks, pessimalShardSelector);
 }
 
-NOINLINE_DECL auto makeChunkManagerWithOptimalBalancedDistribution(int nShards, int nChunks) {
+NOINLINE_DECL auto makeChunkManagerWithOptimalBalancedDistribution(int nShards, uint32_t nChunks) {
     return makeChunkManagerWithShardSelector(nShards, nChunks, optimalShardSelector);
 }
 
@@ -125,7 +125,7 @@ BENCHMARK(BM_IncrementalRefreshOfPessimalBalancedDistribution)->Args({2, 50000})
 template <typename ShardSelectorFn>
 auto BM_FullBuildOfChunkManager(benchmark::State& state, ShardSelectorFn selectShard) {
     const int nShards = state.range(0);
-    const int nChunks = state.range(1);
+    const uint32_t nChunks = state.range(1);
 
     const auto collEpoch = OID::gen();
     const auto collName = NamespaceString("test.foo");
@@ -133,7 +133,7 @@ auto BM_FullBuildOfChunkManager(benchmark::State& state, ShardSelectorFn selectS
 
     std::vector<ChunkType> chunks;
     chunks.reserve(nChunks);
-    for (int i = 0; i < nChunks; ++i) {
+    for (uint32_t i = 0; i < nChunks; ++i) {
         chunks.emplace_back(collName,
                             getRangeForChunk(i, nChunks),
                             ChunkVersion{i + 1, 0, collEpoch},
diff --git a/src/mongo/s/chunk_version.h b/src/mongo/s/chunk_version.h
index 6a045b0b515..441176a9e86 100644
--- a/src/mongo/s/chunk_version.h
+++ b/src/mongo/s/chunk_version.h
@@ -56,7 +56,7 @@ public:
 
     ChunkVersion() : _combined(0), _epoch(OID()) {}
 
-    ChunkVersion(int major, int minor, const OID& epoch)
+    ChunkVersion(uint32_t major, uint32_t minor, const OID& epoch)
         : _combined(static_cast<uint64_t>(minor) | (static_cast<uint64_t>(major) << 32)),
           _epoch(epoch) {}
 
@@ -127,12 +127,12 @@ public:
         return _combined > 0;
     }
 
-    int majorVersion() const {
+    uint32_t majorVersion() const {
         return _combined >> 32;
     }
 
-    int minorVersion() const {
-        return _combined & 0xFFFF;
+    uint32_t minorVersion() const {
+        return _combined & 0xFFFFFFFF;
     }
 
     const OID& epoch() const {
diff --git a/src/mongo/s/chunk_version_test.cpp b/src/mongo/s/chunk_version_test.cpp
index 38512c0df38..977e18d332a 100644
--- a/src/mongo/s/chunk_version_test.cpp
+++ b/src/mongo/s/chunk_version_test.cpp
@@ -28,6 +28,8 @@
 
 #include "mongo/platform/basic.h"
 
+#include <limits>
+
 #include "mongo/s/chunk_version.h"
 #include "mongo/unittest/unittest.h"
 
@@ -70,8 +72,8 @@ TEST(ChunkVersionParsing, FromBSON) {
 
     ASSERT(chunkVersionComplete.epoch().isSet());
     ASSERT_EQ(oid, chunkVersionComplete.epoch());
-    ASSERT_EQ(2, chunkVersionComplete.majorVersion());
-    ASSERT_EQ(3, chunkVersionComplete.minorVersion());
+    ASSERT_EQ(2u, chunkVersionComplete.majorVersion());
+    ASSERT_EQ(3u, chunkVersionComplete.minorVersion());
 }
 
 TEST(ChunkVersionParsing, FromBSONMissingEpoch) {
@@ -97,8 +99,8 @@ TEST(ChunkVersionParsing, FromBSONLegacy) {
 
     ASSERT(chunkVersionComplete.epoch().isSet());
     ASSERT_EQ(oid, chunkVersionComplete.epoch());
-    ASSERT_EQ(2, chunkVersionComplete.majorVersion());
-    ASSERT_EQ(3, chunkVersionComplete.minorVersion());
+    ASSERT_EQ(2u, chunkVersionComplete.majorVersion());
+    ASSERT_EQ(3u, chunkVersionComplete.minorVersion());
 }
 
 TEST(ChunkVersionParsing, FromBSONLegacyEpochIsOptional) {
@@ -106,8 +108,8 @@ TEST(ChunkVersionParsing, FromBSONLegacyEpochIsOptional) {
         ChunkVersion::parseLegacyWithField(BSON("lastmod" << Timestamp(Seconds(3), 4)), "lastmod"));
 
     ASSERT(!chunkVersionNoEpoch.epoch().isSet());
-    ASSERT_EQ(3, chunkVersionNoEpoch.majorVersion());
-    ASSERT_EQ(4, chunkVersionNoEpoch.minorVersion());
+    ASSERT_EQ(3u, chunkVersionNoEpoch.majorVersion());
+    ASSERT_EQ(4u, chunkVersionNoEpoch.minorVersion());
 }
 
 TEST(ChunkVersionComparison, EqualityOperators) {
@@ -138,5 +140,16 @@ TEST(ChunkVersionComparison, OlderThan) {
     ASSERT(!ChunkVersion(3, 1, epoch).isOlderThan(ChunkVersion(3, 1, epoch)));
 }
 
+TEST(ChunkVersionConstruction, CreateWithLargeValues) {
+    const auto minorVersion = std::numeric_limits<uint32_t>::max();
+    const uint32_t majorVersion = 1 << 24;
+    const auto epoch = OID::gen();
+
+    ChunkVersion version(majorVersion, minorVersion, epoch);
+    ASSERT_EQ(majorVersion, version.majorVersion());
+    ASSERT_EQ(minorVersion, version.minorVersion());
+    ASSERT_EQ(epoch, version.epoch());
+}
+
 }  // namespace
 }  // namespace mongo