diff options
author | Matthew Saltz <matthew.saltz@mongodb.com> | 2021-06-24 18:48:37 +0000 |
---|---|---|
committer | Matthew Saltz <matthew.saltz@mongodb.com> | 2021-06-24 21:12:44 +0000 |
commit | 70397073ac3fb14afc19f69bebf0f67cab041a16 (patch) | |
tree | 073fcc05d4e878e13095b85dc0e2121ca317ad10 | |
parent | 174ce8fa6dde67b5c11c702848744c38cf2170bb (diff) | |
download | mongo-70397073ac3fb14afc19f69bebf0f67cab041a16.tar.gz |
SERVER-58023 PrimaryOnlyService should not invariant that _activeInstances is empty when in kPaused
(cherry picked from commit 6e08ac57662b82c3249d2bd2f6a02841f6bf75c5)
-rw-r--r-- | src/mongo/db/repl/primary_only_service.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/primary_only_service_test.cpp | 98 |
2 files changed, 98 insertions, 2 deletions
diff --git a/src/mongo/db/repl/primary_only_service.cpp b/src/mongo/db/repl/primary_only_service.cpp index b95354b5f2e..e7e3779e97f 100644 --- a/src/mongo/db/repl/primary_only_service.cpp +++ b/src/mongo/db/repl/primary_only_service.cpp @@ -542,7 +542,6 @@ boost::optional<std::shared_ptr<PrimaryOnlyService::Instance>> PrimaryOnlyServic _rebuildCV, lk, [this]() { return _state != State::kRebuilding; }); if (_state == State::kShutdown || _state == State::kPaused) { - invariant(_activeInstances.empty()); return boost::none; } if (_state == State::kRebuildFailed) { @@ -573,7 +572,6 @@ std::vector<std::shared_ptr<PrimaryOnlyService::Instance>> PrimaryOnlyService::g _rebuildCV, lk, [this]() { return _state != State::kRebuilding; }); if (_state == State::kShutdown || _state == State::kPaused) { - invariant(_activeInstances.empty()); return instances; } if (_state == State::kRebuildFailed) { diff --git a/src/mongo/db/repl/primary_only_service_test.cpp b/src/mongo/db/repl/primary_only_service_test.cpp index 4aa1798cc9d..e510dbe5d6b 100644 --- a/src/mongo/db/repl/primary_only_service_test.cpp +++ b/src/mongo/db/repl/primary_only_service_test.cpp @@ -27,6 +27,7 @@ * it in the license file. */ +#include <boost/optional/optional_io.hpp> #include <memory> #include "mongo/db/client.h" @@ -90,6 +91,10 @@ public: return std::make_shared<TestService::Instance>(this, std::move(initialState)); } + // Make this public since it's protected in the base class but it needs to be public if we want + // to test it here. + using PrimaryOnlyService::getAllInstances; + class Instance final : public PrimaryOnlyService::TypedInstance<Instance> { public: explicit Instance(const TestService* service, BSONObj stateDoc) @@ -605,6 +610,99 @@ DEATH_TEST_F(PrimaryOnlyServiceTest, } } +TEST_F(PrimaryOnlyServiceTest, LookupInstanceAfterStepDownReturnsNone) { + // Make sure the instance doesn't complete before we try to look it up. + auto timesEntered = TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn); + + auto opCtx = makeOperationContext(); + auto instance = + TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0)); + ASSERT(instance.get()); + ASSERT_EQ(0, instance->getID()); + + // Wait for Instance::run() to be called before calling stepDown so that the _completionPromise + // will eventually be set. + TestServiceHangDuringInitialization.waitForTimesEntered(timesEntered + 1); + + stepDown(); + + auto instance2 = TestService::Instance::lookup(opCtx.get(), _service, BSON("_id" << 0)); + + ASSERT_EQ(instance2, boost::none); + + TestServiceHangDuringInitialization.setMode(FailPoint::off); + ASSERT_EQ(ErrorCodes::Interrupted, instance->getCompletionFuture().getNoThrow()); +} + +TEST_F(PrimaryOnlyServiceTest, LookupInstanceAfterShutDownReturnsNone) { + // Make sure the instance doesn't complete before we try to look it up. + auto timesEntered = TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn); + + auto opCtx = makeOperationContext(); + auto instance = + TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0)); + ASSERT(instance.get()); + ASSERT_EQ(0, instance->getID()); + // Make sure we enter the run function before shutting down. + TestServiceHangDuringInitialization.waitForTimesEntered(timesEntered + 1); + TestServiceHangDuringInitialization.setMode(FailPoint::off); + + shutdown(); + + auto instance2 = TestService::Instance::lookup(opCtx.get(), _service, BSON("_id" << 0)); + + ASSERT_EQ(instance2, boost::none); + + ASSERT_EQ(ErrorCodes::Interrupted, instance->getCompletionFuture().getNoThrow()); +} + +TEST_F(PrimaryOnlyServiceTest, GetAllInstancesAfterStepDownReturnsEmptyVector) { + // Make sure the instance doesn't complete before we try to look it up. + auto timesEntered = TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn); + + auto opCtx = makeOperationContext(); + auto instance = + TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0)); + ASSERT(instance.get()); + ASSERT_EQ(0, instance->getID()); + + // Wait for Instance::run() to be called before calling stepDown so that the _completionPromise + // will eventually be set. + TestServiceHangDuringInitialization.waitForTimesEntered(timesEntered + 1); + + stepDown(); + + auto instances = static_cast<TestService*>(_service)->getAllInstances(opCtx.get()); + + ASSERT_EQ(instances.size(), 0); + + TestServiceHangDuringInitialization.setMode(FailPoint::off); + ASSERT_EQ(ErrorCodes::Interrupted, instance->getCompletionFuture().getNoThrow()); +} + +TEST_F(PrimaryOnlyServiceTest, GetAllInstancesAfterShutDownReturnsEmptyVector) { + // Make sure the instance doesn't complete before we try to look it up. + auto timesEntered = TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn); + + auto opCtx = makeOperationContext(); + auto instance = + TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0)); + ASSERT(instance.get()); + ASSERT_EQ(0, instance->getID()); + + // Make sure we enter the run function before shutting down. + TestServiceHangDuringInitialization.waitForTimesEntered(timesEntered + 1); + TestServiceHangDuringInitialization.setMode(FailPoint::off); + + shutdown(); + + auto instances = static_cast<TestService*>(_service)->getAllInstances(opCtx.get()); + + ASSERT_EQ(instances.size(), 0); + + ASSERT_EQ(ErrorCodes::Interrupted, instance->getCompletionFuture().getNoThrow()); +} + TEST_F(PrimaryOnlyServiceTest, GetOrCreateInstanceInterruptible) { stepDown(); // Make sure the service stays in state kRebuilding so that getOrCreate() has to try to wait on |