summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mongo/db/s/metadata_manager.cpp3
-rw-r--r--src/mongo/db/s/range_deletion_util.cpp13
-rw-r--r--src/mongo/db/s/range_deletion_util.h4
-rw-r--r--src/mongo/db/s/range_deletion_util_test.cpp91
4 files changed, 46 insertions, 65 deletions
diff --git a/src/mongo/db/s/metadata_manager.cpp b/src/mongo/db/s/metadata_manager.cpp
index ab79f546fd1..1d504054026 100644
--- a/src/mongo/db/s/metadata_manager.cpp
+++ b/src/mongo/db/s/metadata_manager.cpp
@@ -507,8 +507,7 @@ SharedSemiFuture<void> MetadataManager::_submitRangeForDeletion(
range,
std::move(migrationId),
maxToDelete,
- delayForActiveQueriesOnSecondariesToComplete,
- Milliseconds(rangeDeleterBatchDelayMS.load()));
+ delayForActiveQueriesOnSecondariesToComplete);
_rangesScheduledForDeletion.emplace_front(range, cleanupComplete);
// Attach a continuation so that once the range has been deleted, we will remove the deletion
diff --git a/src/mongo/db/s/range_deletion_util.cpp b/src/mongo/db/s/range_deletion_util.cpp
index 0e26c3aaff0..9a4263c4005 100644
--- a/src/mongo/db/s/range_deletion_util.cpp
+++ b/src/mongo/db/s/range_deletion_util.cpp
@@ -350,6 +350,13 @@ ExecutorFuture<void> deleteRangeInBatches(const std::shared_ptr<executor::TaskEx
"collectionUUID"_attr = collectionUuid,
"range"_attr = range.toString());
+ if (numDeleted > 0) {
+ // (SERVER-62368) The range-deleter executor is mono-threaded, so
+ // sleeping synchronously for `delayBetweenBatches` ensures that no other
+ // batch is going to be cleared up before the expected delay.
+ opCtx->sleepFor(delayBetweenBatches);
+ }
+
return numDeleted;
},
nss);
@@ -365,7 +372,6 @@ ExecutorFuture<void> deleteRangeInBatches(const std::shared_ptr<executor::TaskEx
ErrorCodes::isShutdownError(swNumDeleted.getStatus()) ||
ErrorCodes::isNotPrimaryError(swNumDeleted.getStatus());
})
- .withDelayBetweenIterations(delayBetweenBatches)
.on(executor)
.ignoreValue();
}
@@ -440,8 +446,7 @@ SharedSemiFuture<void> removeDocumentsInRange(
const ChunkRange& range,
boost::optional<UUID> migrationId,
int numDocsToRemovePerBatch,
- Seconds delayForActiveQueriesOnSecondariesToComplete,
- Milliseconds delayBetweenBatches) {
+ Seconds delayForActiveQueriesOnSecondariesToComplete) {
return std::move(waitForActiveQueriesToComplete)
.thenRunOn(executor)
.onError([&](Status s) {
@@ -498,7 +503,7 @@ SharedSemiFuture<void> removeDocumentsInRange(
range,
migrationId,
numDocsToRemovePerBatch,
- delayBetweenBatches)
+ Milliseconds(rangeDeleterBatchDelayMS.load()))
.onCompletion([=](Status s) {
if (!s.isOK() &&
s.code() !=
diff --git a/src/mongo/db/s/range_deletion_util.h b/src/mongo/db/s/range_deletion_util.h
index a1f66b78578..acd6852aef3 100644
--- a/src/mongo/db/s/range_deletion_util.h
+++ b/src/mongo/db/s/range_deletion_util.h
@@ -72,6 +72,6 @@ SharedSemiFuture<void> removeDocumentsInRange(
const ChunkRange& range,
boost::optional<UUID> migrationId,
int numDocsToRemovePerBatch,
- Seconds delayForActiveQueriesOnSecondariesToComplete,
- Milliseconds delayBetweenBatches);
+ Seconds delayForActiveQueriesOnSecondariesToComplete);
+
} // namespace mongo
diff --git a/src/mongo/db/s/range_deletion_util_test.cpp b/src/mongo/db/s/range_deletion_util_test.cpp
index 8c5b5adc797..16f99c4cebf 100644
--- a/src/mongo/db/s/range_deletion_util_test.cpp
+++ b/src/mongo/db/s/range_deletion_util_test.cpp
@@ -149,8 +149,7 @@ TEST_F(RangeDeleterTest,
range,
boost::none,
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
ASSERT_EQUALS(dbclient.count(kNss, BSONObj()), 0);
@@ -179,8 +178,7 @@ TEST_F(RangeDeleterTest,
range,
boost::none,
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
ASSERT_EQUALS(dbclient.count(kNss, BSONObj()), 0);
@@ -203,8 +201,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeInsertsDocumentToNotifySecondarie
range,
boost::none,
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
@@ -237,8 +234,7 @@ TEST_F(
range,
boost::none,
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
ASSERT_EQUALS(dbclient.count(NamespaceString::kServerConfigurationNamespace,
@@ -270,8 +266,7 @@ TEST_F(RangeDeleterTest,
range,
boost::none,
1 /* numDocsToRemovePerBatch */,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
// No documents should have been deleted.
@@ -302,8 +297,7 @@ TEST_F(RangeDeleterTest,
range,
boost::none,
1 /* numDocsToRemovePerBatch */,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
// No documents should have been deleted.
@@ -330,8 +324,7 @@ TEST_F(RangeDeleterTest,
ChunkRange(BSON(kShardKey << 0), BSON(kShardKey << 10)),
boost::none,
10 /* numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
ASSERT_THROWS_CODE(cleanupComplete.get(),
@@ -351,8 +344,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeThrowsErrorWhenCollectionDoesNotE
ChunkRange(BSON(kShardKey << 0), BSON(kShardKey << 10)),
boost::none,
10 /* numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
ASSERT_THROWS_CODE(cleanupComplete.get(),
@@ -395,8 +387,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeLeavesDocumentsWhenTaskDocumentDo
range,
UUID::gen(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
@@ -454,8 +445,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeWaitsForReplicationAfterDeletingS
range,
t.getId(),
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
cleanupComplete.get();
@@ -513,8 +503,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeWaitsForReplicationOnlyOnceAfterS
range,
t.getId(),
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
@@ -570,8 +559,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeDoesNotWaitForReplicationIfErrorD
range,
t.getId(),
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
ASSERT_THROWS_CODE(cleanupComplete.get(), DBException, ErrorCodes::PrimarySteppedDown);
ASSERT_EQ(numTimesWaitedForReplication, 0);
@@ -610,8 +598,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRetriesOnWriteConflictException)
range,
t.getId(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
@@ -651,8 +638,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRetriesOnUnexpectedError) {
range,
t.getId(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
@@ -665,15 +651,22 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRespectsDelayInBetweenBatches) {
// More documents than the batch size.
const auto numDocsToInsert = 3;
const auto numDocsToRemovePerBatch = 1;
- const auto delayBetweenBatches = Milliseconds(10);
auto queriesComplete = SemiFuture<void>::makeReady();
-
// Insert documents in range.
DBDirectClient dbclient(operationContext());
for (auto i = 0; i < numDocsToInsert; ++i) {
dbclient.insert(kNss.toString(), BSON(kShardKey << i));
}
+ // The deletion of a document in unit tests with ephemeral storage engine is usually
+ // extremely fast (less than 5ms), so setting the delay to 1 second ensures the test
+ // is relevant: it is very improbable for a deletion to last so much, even on slow
+ // machines.
+ const auto delayBetweenBatchesMS = 1000 /* 1 second */;
+ rangeDeleterBatchDelayMS.store(delayBetweenBatchesMS);
+
+ auto beforeRangeDeletion = Date_t::now();
+
auto cleanupComplete =
removeDocumentsInRange(executor(),
std::move(queriesComplete),
@@ -683,23 +676,13 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRespectsDelayInBetweenBatches) {
range,
boost::none,
numDocsToRemovePerBatch,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- delayBetweenBatches);
-
- // A best-effort check that cleanup has not completed without advancing the clock.
- sleepsecs(1);
- ASSERT_FALSE(cleanupComplete.isReady());
-
- // Advance the time until cleanup is complete. This explicit advancement of the clock is
- // required in order to allow the delay between batches to complete. This cannot be made exact
- // because there's no way to tell when the sleep operation gets hit exactly, so instead we
- // incrementally advance time until it's ready.
- while (!cleanupComplete.isReady()) {
- executor::NetworkInterfaceMock::InNetworkGuard guard(network());
- network()->advanceTime(network()->now() + Milliseconds(1));
- }
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
+ auto afterRangeDeletion = Date_t::now();
+ auto rangeDeletionTimeMS =
+ afterRangeDeletion.toMillisSinceEpoch() - beforeRangeDeletion.toMillisSinceEpoch();
+ ASSERT(rangeDeletionTimeMS >= delayBetweenBatchesMS * numDocsToInsert);
ASSERT_EQUALS(dbclient.count(kNss, BSONObj()), 0);
}
@@ -725,8 +708,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRespectsOrphanCleanupDelay) {
range,
boost::none,
numDocsToRemovePerBatch,
- orphanCleanupDelay,
- Milliseconds(0) /* delayBetweenBatches */);
+ orphanCleanupDelay);
// A best-effort check that cleanup has not completed without advancing the clock.
sleepsecs(1);
@@ -775,8 +757,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeRemovesRangeDeletionTaskOnSuccess
range,
t.getId(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
cleanupComplete.get();
// Document should have been deleted.
@@ -815,8 +796,7 @@ TEST_F(RangeDeleterTest,
range,
t.getId(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
ASSERT_THROWS_CODE(cleanupComplete.get(),
DBException,
@@ -862,8 +842,7 @@ TEST_F(RangeDeleterTest,
range,
t.getId(),
10 /*numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
ASSERT_THROWS_CODE(cleanupComplete.get(), DBException, ErrorCodes::PrimarySteppedDown);
@@ -890,8 +869,7 @@ DEATH_TEST_F(RangeDeleterTest, RemoveDocumentsInRangeCrashesIfInputFutureHasErro
ChunkRange(BSON(kShardKey << 0), BSON(kShardKey << 10)),
boost::none,
10 /* numDocsToRemovePerBatch */,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete */);
// Should cause an invariant failure.
@@ -913,8 +891,7 @@ TEST_F(RangeDeleterTest, RemoveDocumentsInRangeDoesNotCrashWhenShardKeyIndexDoes
ChunkRange(BSON("x" << 0), BSON("x" << 10)),
boost::none,
10 /* numDocsToRemovePerBatch*/,
- Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/,
- Milliseconds(0) /* delayBetweenBatches */);
+ Seconds(0) /* delayForActiveQueriesOnSecondariesToComplete*/);
// Range deleter will keep on retrying when it encounters non-stepdown errors. Make it run
// a few iterations and then create the index to make it exit the retry loop.
View Lorry Controller Status