diff options
author | Alex Taskov <alex.taskov@mongodb.com> | 2019-06-05 14:41:05 -0400 |
---|---|---|
committer | Alex Taskov <alex.taskov@mongodb.com> | 2019-06-10 17:45:46 -0400 |
commit | ece39c7650b76a45ee2357dc4a7adb2ab195eda4 (patch) | |
tree | 7fc987f88ab3d8e2d8ab01a6f1858672f2dd5c58 | |
parent | e297640646d2b38401698ab5e77c3770614b6d69 (diff) | |
download | mongo-ece39c7650b76a45ee2357dc4a7adb2ab195eda4.tar.gz |
SERVER-39141 Handle WriteConflictException in range deleter
-rw-r--r-- | src/mongo/db/s/collection_range_deleter.cpp | 33 | ||||
-rw-r--r-- | src/mongo/db/s/collection_range_deleter.h | 8 | ||||
-rw-r--r-- | src/mongo/db/s/collection_range_deleter_test.cpp | 29 |
3 files changed, 60 insertions, 10 deletions
diff --git a/src/mongo/db/s/collection_range_deleter.cpp b/src/mongo/db/s/collection_range_deleter.cpp index e1bd537700c..1b63c1ce74c 100644 --- a/src/mongo/db/s/collection_range_deleter.cpp +++ b/src/mongo/db/s/collection_range_deleter.cpp @@ -112,7 +112,7 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange( } } - StatusWith<int> wrote = 0; + StatusWith<int> swNumDeleted = 0; auto range = boost::optional<ChunkRange>(boost::none); auto notification = DeleteNotification(); @@ -201,22 +201,31 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange( const auto& metadata = *scopedCollectionMetadata; try { - wrote = self->_doDeletion( + swNumDeleted = self->_doDeletion( opCtx, collection, metadata->getKeyPattern(), *range, maxToDelete); } catch (const DBException& e) { - wrote = e.toStatus(); + swNumDeleted = e.toStatus(); warning() << e.what(); } } // drop autoColl - if (!wrote.isOK() || wrote.getValue() == 0) { - if (wrote.isOK()) { + bool continueDeleting = swNumDeleted.isOK() && swNumDeleted.getValue() > 0; + + if (swNumDeleted == ErrorCodes::WriteConflict) { + CurOp::get(opCtx)->debug().additiveMetrics.incrementWriteConflicts(1); + continueDeleting = true; + } + + // If there's an error or if there are no more documents to delete, take this branch. + // This branch means that we will NOT continue deleting documents from this range. + if (!continueDeleting) { + if (swNumDeleted.isOK()) { LOG(0) << "No documents remain to delete in " << nss << " range " << redact(range->toString()); } - // Wait for majority replication even when wrote isn't OK or == 0, because it might have - // been OK and/or > 0 previously, and the deletions must be persistent before notifying + // Wait for majority replication even when swNumDeleted isn't OK or == 0, because it might + // have been OK and/or > 0 previously, and the deletions must be persistent before notifying // clients in _pop(). LOG(0) << "Waiting for majority replication of local deletions in " << nss.ns() << " range " @@ -270,7 +279,7 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange( LOG(0) << "Finished deleting documents in " << nss.ns() << " range " << redact(range->toString()); - self->_pop(wrote.getStatus()); + self->_pop(swNumDeleted.getStatus()); } if (!self->_orphans.empty()) { @@ -282,8 +291,7 @@ boost::optional<Date_t> CollectionRangeDeleter::cleanUpNextRange( } invariant(range); - invariant(wrote.getStatus()); - invariant(wrote.getValue() > 0); + invariant(continueDeleting); notification.abandon(); return Date_t::now() + Milliseconds(rangeDeleterBatchDelayMS.load()); @@ -403,6 +411,11 @@ StatusWith<int> CollectionRangeDeleter::_doDeletion(OperationContext* opCtx, int numDeleted = 0; do { BSONObj deletedObj; + + // TODO SERVER-41606: Remove this function when we refactor CollectionRangeDeleter. + if (_throwWriteConflictForTest) + throw WriteConflictException(); + PlanExecutor::ExecState state = exec->getNext(&deletedObj, nullptr); if (state == PlanExecutor::IS_EOF) { diff --git a/src/mongo/db/s/collection_range_deleter.h b/src/mongo/db/s/collection_range_deleter.h index 9de86f4c694..6fae0ee5d18 100644 --- a/src/mongo/db/s/collection_range_deleter.h +++ b/src/mongo/db/s/collection_range_deleter.h @@ -183,6 +183,11 @@ public: int maxToDelete = 0, CollectionRangeDeleter* forTestOnly = nullptr); + // TODO SERVER-41606: Remove this function when we refactor CollectionRangeDeleter. + void setDoDeletionShouldThrowWriteConflictForTest(bool on) { + _throwWriteConflictForTest = on; + } + private: /** * Verifies that the metadata for the collection to be cleaned up is still valid. Makes sure @@ -214,6 +219,9 @@ private: */ void _pop(Status status); + // TODO SERVER-41606: Remove this function when we refactor CollectionRangeDeleter. + bool _throwWriteConflictForTest{false}; + /** * Ranges scheduled for deletion. The front of the list will be in active process of deletion. * As each range is completed, its notification is signaled before it is popped. diff --git a/src/mongo/db/s/collection_range_deleter_test.cpp b/src/mongo/db/s/collection_range_deleter_test.cpp index 320b6b3796b..f6f6beb43c6 100644 --- a/src/mongo/db/s/collection_range_deleter_test.cpp +++ b/src/mongo/db/s/collection_range_deleter_test.cpp @@ -385,5 +385,34 @@ TEST_F(CollectionRangeDeleterTest, MultipleDocumentsInMultipleRangesToClean) { ASSERT_FALSE(next(rangeDeleter, 1)); } +// Tests that we retry on a WriteConflictException. +TEST_F(CollectionRangeDeleterTest, RetryOnWriteConflictException) { + CollectionRangeDeleter rangeDeleter; + DBDirectClient dbclient(operationContext()); + + dbclient.insert(kNss.toString(), BSON(kShardKey << 1)); + dbclient.insert(kNss.toString(), BSON(kShardKey << 2)); + dbclient.insert(kNss.toString(), BSON(kShardKey << 3)); + ASSERT_EQUALS(3ULL, dbclient.count(kNss.toString(), BSON(kShardKey << LT << 5))); + + std::list<Deletion> ranges; + auto deletion = Deletion{ChunkRange(BSON(kShardKey << 0), BSON(kShardKey << 10)), Date_t{}}; + ranges.emplace_back(std::move(deletion)); + auto when = rangeDeleter.add(std::move(ranges)); + ASSERT(when && *when == Date_t{}); + + // TODO SERVER-41606: Remove this function when we refactor CollectionRangeDeleter. + rangeDeleter.setDoDeletionShouldThrowWriteConflictForTest(true); + + ASSERT_TRUE(next(rangeDeleter, 1)); + ASSERT_EQUALS(3ULL, dbclient.count(kNss.toString(), BSON(kShardKey << LT << 5))); + + // TODO SERVER-41606: Remove this function when we refactor CollectionRangeDeleter. + rangeDeleter.setDoDeletionShouldThrowWriteConflictForTest(false); + + ASSERT_TRUE(next(rangeDeleter, 1)); + ASSERT_EQUALS(2ULL, dbclient.count(kNss.toString(), BSON(kShardKey << LT << 5))); +} + } // namespace } // namespace mongo |