diff options
author | Tommaso Tocci <tommaso.tocci@10gen.com> | 2020-01-02 14:03:32 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2020-01-02 14:03:32 +0000 |
commit | 45bf534466eb5e82dcb71e8bb7c7f1791b759276 (patch) | |
tree | e38a394f1854404dfdb63b8008652862f017d205 | |
parent | c92e20479618b22355b9fb7efa935ff6db5883a9 (diff) | |
download | mongo-45bf534466eb5e82dcb71e8bb7c7f1791b759276.tar.gz |
SERVER-14052 Remove reduntant param in splitVector func
The internal splitVector function was accepting both
- maxChunkSize
- maxChunkSizeBytes
The first one has been removed because is obviusly redundant.
The SplitVector command still accepts both of them and for compatibility
reason maxChunkSize has precedence over maxChunkSizeBytes over there.
-rw-r--r-- | src/mongo/db/s/chunk_splitter.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/s/split_vector.cpp | 23 | ||||
-rw-r--r-- | src/mongo/db/s/split_vector.h | 1 | ||||
-rw-r--r-- | src/mongo/db/s/split_vector_command.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/s/split_vector_test.cpp | 28 |
5 files changed, 14 insertions, 54 deletions
diff --git a/src/mongo/db/s/chunk_splitter.cpp b/src/mongo/db/s/chunk_splitter.cpp index 33235a3be68..f49d08743a7 100644 --- a/src/mongo/db/s/chunk_splitter.cpp +++ b/src/mongo/db/s/chunk_splitter.cpp @@ -325,7 +325,6 @@ void ChunkSplitter::_runAutosplit(std::shared_ptr<ChunkSplitStateDriver> chunkSp false, boost::none, boost::none, - boost::none, maxChunkSizeBytes)); if (splitPoints.empty()) { diff --git a/src/mongo/db/s/split_vector.cpp b/src/mongo/db/s/split_vector.cpp index 14ad3ecdc34..6b257208da1 100644 --- a/src/mongo/db/s/split_vector.cpp +++ b/src/mongo/db/s/split_vector.cpp @@ -66,7 +66,6 @@ StatusWith<std::vector<BSONObj>> splitVector(OperationContext* opCtx, bool force, boost::optional<long long> maxSplitPoints, boost::optional<long long> maxChunkObjects, - boost::optional<long long> maxChunkSize, boost::optional<long long> maxChunkSizeBytes) { std::vector<BSONObj> splitKeys; std::size_t splitVectorResponseSize = 0; @@ -113,26 +112,20 @@ StatusWith<std::vector<BSONObj>> splitVector(OperationContext* opCtx, // Now that we have the size estimate, go over the remaining parameters and apply any // maximum size restrictions specified there. - // Forcing a split is equivalent to having maxChunkSize be the size of the current + // Forcing a split is equivalent to having maxChunkSizeBytes be the size of the current // chunk, i.e., the logic below will split that chunk in half if (force) { - maxChunkSize = dataSize; - } else if (!maxChunkSize) { - if (maxChunkSizeBytes) { - maxChunkSize = maxChunkSizeBytes.get(); - } - } else { - maxChunkSize = maxChunkSize.get() * 1 << 20; + maxChunkSizeBytes = dataSize; } // We need a maximum size for the chunk. - if (!maxChunkSize || maxChunkSize.get() <= 0) { + if (!maxChunkSizeBytes || maxChunkSizeBytes.get() <= 0) { return {ErrorCodes::InvalidOptions, "need to specify the desired max chunk size"}; } // If there's not enough data for more than one chunk, no point continuing. - if (dataSize < maxChunkSize.get() || recCount == 0) { + if (dataSize < maxChunkSizeBytes.get() || recCount == 0) { std::vector<BSONObj> emptyVector; return emptyVector; } @@ -141,11 +134,11 @@ StatusWith<std::vector<BSONObj>> splitVector(OperationContext* opCtx, << " -->> " << redact(maxKey); // We'll use the average object size and number of object to find approximately how many - // keys each chunk should have. We'll split at half the maxChunkSize or maxChunkObjects, - // if provided. + // keys each chunk should have. We'll split at half the maxChunkSizeBytes or + // maxChunkObjects, if provided. const long long avgRecSize = dataSize / recCount; - long long keyCount = maxChunkSize.get() / (2 * avgRecSize); + long long keyCount = maxChunkSizeBytes.get() / (2 * avgRecSize); if (maxChunkObjects.get() && (maxChunkObjects.get() < keyCount)) { log() << "limiting split vector to " << maxChunkObjects.get() << " (from " << keyCount @@ -299,7 +292,7 @@ StatusWith<std::vector<BSONObj>> splitVector(OperationContext* opCtx, // index // - // Warn for keys that are more numerous than maxChunkSize allows. + // Warn for keys that are more numerous than maxChunkSizeBytes allows. for (auto it = tooFrequentKeys.cbegin(); it != tooFrequentKeys.cend(); ++it) { warning() << "possible low cardinality key detected in " << nss.toString() << " - key is " << redact(prettyKey(idx->keyPattern(), *it)); diff --git a/src/mongo/db/s/split_vector.h b/src/mongo/db/s/split_vector.h index d9343a65371..9429dff6a8b 100644 --- a/src/mongo/db/s/split_vector.h +++ b/src/mongo/db/s/split_vector.h @@ -64,7 +64,6 @@ StatusWith<std::vector<BSONObj>> splitVector(OperationContext* opCtx, bool force, boost::optional<long long> maxSplitPoints, boost::optional<long long> maxChunkObjects, - boost::optional<long long> maxChunkSize, boost::optional<long long> maxChunkSizeBytes); } // namespace mongo diff --git a/src/mongo/db/s/split_vector_command.cpp b/src/mongo/db/s/split_vector_command.cpp index 0b65dc3fc14..9af279ce4ef 100644 --- a/src/mongo/db/s/split_vector_command.cpp +++ b/src/mongo/db/s/split_vector_command.cpp @@ -123,16 +123,14 @@ public: maxChunkObjects = maxChunkObjectsElem.numberLong(); } - boost::optional<long long> maxChunkSize; - BSONElement maxSizeElem = jsobj["maxChunkSize"]; - if (maxSizeElem.isNumber()) { - maxChunkSize = maxSizeElem.numberLong(); - } - boost::optional<long long> maxChunkSizeBytes; - maxSizeElem = jsobj["maxChunkSizeBytes"]; + BSONElement maxSizeElem = jsobj["maxChunkSize"]; + BSONElement maxSizeBytesElem = jsobj["maxChunkSizeBytes"]; + // Use maxChunkSize if present otherwise maxChunkSizeBytes if (maxSizeElem.isNumber()) { - maxChunkSizeBytes = maxSizeElem.numberLong(); + maxChunkSizeBytes = maxSizeElem.numberLong() * 1 << 20; + } else if (maxSizeBytesElem.isNumber()) { + maxChunkSizeBytes = maxSizeBytesElem.numberLong(); } auto statusWithSplitKeys = splitVector(opCtx, @@ -143,7 +141,6 @@ public: force, maxSplitPoints, maxChunkObjects, - maxChunkSize, maxChunkSizeBytes); uassertStatusOK(statusWithSplitKeys.getStatus()); diff --git a/src/mongo/db/s/split_vector_test.cpp b/src/mongo/db/s/split_vector_test.cpp index 22e226ff4ce..75fdfb271e6 100644 --- a/src/mongo/db/s/split_vector_test.cpp +++ b/src/mongo/db/s/split_vector_test.cpp @@ -80,7 +80,6 @@ TEST_F(SplitVectorTest, SplitVectorInHalf) { false, boost::none, boost::none, - boost::none, getDocSizeBytes() * 100LL)); std::vector<BSONObj> expected = {BSON(kPattern << 50)}; ASSERT_EQ(splitKeys.size(), expected.size()); @@ -101,7 +100,6 @@ TEST_F(SplitVectorTest, ForceSplit) { true, boost::none, boost::none, - getDocSizeBytes() * 6LL, getDocSizeBytes() * 6LL)); std::vector<BSONObj> expected = {BSON(kPattern << 50)}; ASSERT_EQ(splitKeys.size(), expected.size()); @@ -122,7 +120,6 @@ TEST_F(SplitVectorTest, MaxChunkObjectsSet) { false, boost::none, 10, - boost::none, getDocSizeBytes() * 100LL)); // Unlike the SplitVectorInHalf test, should split at every 10th key. std::vector<BSONObj> expected = {BSON(kPattern << 10), @@ -152,7 +149,6 @@ TEST_F(SplitVectorTest, SplitEveryThird) { false, boost::none, boost::none, - boost::none, getDocSizeBytes() * 6LL)); std::vector<BSONObj> expected = { BSON(kPattern << 3), BSON(kPattern << 7), BSON(kPattern << 11), BSON(kPattern << 15), @@ -180,7 +176,6 @@ TEST_F(SplitVectorTest, MaxSplitPointsSet) { false, 3, boost::none, - boost::none, getDocSizeBytes() * 6LL)); // Unlike the SplitEveryThird test, should only return the first 3 split points since // maxSplitPoints is 3. @@ -204,7 +199,6 @@ TEST_F(SplitVectorTest, IgnoreMaxChunkObjects) { false, boost::none, 10, - boost::none, getDocSizeBytes() * 6LL)); // The "maxChunkObjects"th key (10) is larger than the key count at half the maxChunkSize (3), // so it should be ignored. @@ -234,28 +228,11 @@ TEST_F(SplitVectorTest, NoSplit) { false, boost::none, boost::none, - boost::none, getDocSizeBytes() * 1000LL)); ASSERT_EQUALS(splitKeys.size(), 0UL); } -TEST_F(SplitVectorTest, MaxChunkSizeSpecified) { - std::vector<BSONObj> splitKeys = unittest::assertGet(splitVector(operationContext(), - kNss, - BSON(kPattern << 1), - BSON(kPattern << 0), - BSON(kPattern << 100), - false, - boost::none, - boost::none, - 1LL, - getDocSizeBytes() * 6LL)); - // If both maxChunkSize and maxChunkSizeBytes are specified, maxChunkSize takes precedence. - // Since the size of the collection is not yet a megabyte, should not split. - ASSERT_EQ(splitKeys.size(), 0UL); -} - TEST_F(SplitVectorTest, NoCollection) { auto status = splitVector(operationContext(), NamespaceString("dummy", "collection"), @@ -265,7 +242,6 @@ TEST_F(SplitVectorTest, NoCollection) { false, boost::none, boost::none, - boost::none, boost::none) .getStatus(); ASSERT_EQUALS(status.code(), ErrorCodes::NamespaceNotFound); @@ -280,7 +256,6 @@ TEST_F(SplitVectorTest, NoIndex) { false, boost::none, boost::none, - boost::none, boost::none) .getStatus(); ASSERT_EQUALS(status.code(), ErrorCodes::IndexNotFound); @@ -295,7 +270,6 @@ TEST_F(SplitVectorTest, NoMaxChunkSize) { false, boost::none, boost::none, - boost::none, boost::none) .getStatus(); ASSERT_EQUALS(status.code(), ErrorCodes::InvalidOptions); @@ -341,7 +315,6 @@ TEST_F(SplitVectorJumboTest, JumboChunk) { false, boost::none, boost::none, - boost::none, getDocSizeBytes() * 1LL)); ASSERT_EQUALS(splitKeys.size(), 0UL); @@ -399,7 +372,6 @@ TEST_F(SplitVectorMaxResponseSizeTest, MaxResponseSize) { false, boost::none, boost::none, - boost::none, 1LL)); ASSERT_EQUALS((int)splitKeys.size(), numDocs - 2); |