From 10ecbbca3451433e296fa617d34110965872c2c0 Mon Sep 17 00:00:00 2001 From: Spencer T Brody Date: Wed, 2 Sep 2020 13:49:54 -0400 Subject: SERVER-50700 Allow constructing a FailPointEnableBlock from a pointer to the FailPoint --- src/mongo/util/fail_point.cpp | 35 ++++++++++++--------- src/mongo/util/fail_point.h | 33 +++++++++++++------- src/mongo/util/fail_point_test.cpp | 62 +++++++++++++++++++++++--------------- 3 files changed, 81 insertions(+), 49 deletions(-) diff --git a/src/mongo/util/fail_point.cpp b/src/mongo/util/fail_point.cpp index c0a28ddb3ac..e467ff2d9fb 100644 --- a/src/mongo/util/fail_point.cpp +++ b/src/mongo/util/fail_point.cpp @@ -70,7 +70,7 @@ void FailPoint::setThreadPRNGSeed(int32_t seed) { threadPrng = PseudoRandom(seed); } -FailPoint::FailPoint() = default; +FailPoint::FailPoint(std::string name) : _name(std::move(name)) {} void FailPoint::_shouldFailCloseBlock() { _fpInfo.subtractAndFetch(1); @@ -286,8 +286,8 @@ BSONObj FailPoint::toBSON() const { return builder.obj(); } -FailPointRegisterer::FailPointRegisterer(const std::string& name, FailPoint* fp) { - uassertStatusOK(globalFailPointRegistry().add(name, fp)); +FailPointRegisterer::FailPointRegisterer(FailPoint* fp) { + uassertStatusOK(globalFailPointRegistry().add(fp)); } FailPointRegistry& globalFailPointRegistry() { @@ -309,12 +309,18 @@ auto setGlobalFailPoint(const std::string& failPointName, const BSONObj& cmdObj) return timesEntered; } -FailPointEnableBlock::FailPointEnableBlock(std::string failPointName) - : FailPointEnableBlock(std::move(failPointName), {}) {} +FailPointEnableBlock::FailPointEnableBlock(StringData failPointName) + : FailPointEnableBlock(failPointName, {}) {} + +FailPointEnableBlock::FailPointEnableBlock(StringData failPointName, BSONObj data) + : FailPointEnableBlock(globalFailPointRegistry().find(failPointName), std::move(data)) {} + +FailPointEnableBlock::FailPointEnableBlock(FailPoint* failPoint) + : FailPointEnableBlock(failPoint, {}) {} + +FailPointEnableBlock::FailPointEnableBlock(FailPoint* failPoint, BSONObj data) + : _failPoint(failPoint) { -FailPointEnableBlock::FailPointEnableBlock(std::string failPointName, BSONObj data) - : _failPointName(std::move(failPointName)) { - _failPoint = globalFailPointRegistry().find(_failPointName); invariant(_failPoint != nullptr); _initialTimesEntered = _failPoint->setMode(FailPoint::alwaysOn, 0, std::move(data)); @@ -322,7 +328,7 @@ FailPointEnableBlock::FailPointEnableBlock(std::string failPointName, BSONObj da LOGV2_WARNING(23830, "Set failpoint {failPointName} to: {failPoint}", "Set failpoint", - "failPointName"_attr = _failPointName, + "failPointName"_attr = _failPoint->getName(), "failPoint"_attr = _failPoint->toBSON()); } @@ -331,24 +337,25 @@ FailPointEnableBlock::~FailPointEnableBlock() { LOGV2_WARNING(23831, "Set failpoint {failPointName} to: {failPoint}", "Set failpoint", - "failPointName"_attr = _failPointName, + "failPointName"_attr = _failPoint->getName(), "failPoint"_attr = _failPoint->toBSON()); } FailPointRegistry::FailPointRegistry() : _frozen(false) {} -Status FailPointRegistry::add(const std::string& name, FailPoint* failPoint) { +Status FailPointRegistry::add(FailPoint* failPoint) { if (_frozen) { return {ErrorCodes::CannotMutateObject, "Registry is already frozen"}; } - auto [pos, ok] = _fpMap.insert({name, failPoint}); + auto [pos, ok] = _fpMap.insert({failPoint->getName(), failPoint}); if (!ok) { - return {ErrorCodes::Error(51006), "Fail point already registered: {}"_format(name)}; + return {ErrorCodes::Error(51006), + "Fail point already registered: {}"_format(failPoint->getName())}; } return Status::OK(); } -FailPoint* FailPointRegistry::find(const std::string& name) const { +FailPoint* FailPointRegistry::find(StringData name) const { auto iter = _fpMap.find(name); return (iter == _fpMap.end()) ? nullptr : iter->second; } diff --git a/src/mongo/util/fail_point.h b/src/mongo/util/fail_point.h index 5322fad8b67..af02e9f1622 100644 --- a/src/mongo/util/fail_point.h +++ b/src/mongo/util/fail_point.h @@ -40,6 +40,7 @@ #include "mongo/stdx/unordered_map.h" #include "mongo/util/duration.h" #include "mongo/util/interruptible.h" +#include "mongo/util/string_map.h" namespace mongo { @@ -202,11 +203,15 @@ public: */ static StatusWith parseBSON(const BSONObj& obj); - FailPoint(); + explicit FailPoint(std::string name); FailPoint(const FailPoint&) = delete; FailPoint& operator=(const FailPoint&) = delete; + const std::string& getName() const { + return _name; + } + /** * Returns true if fail point is active. * @@ -423,6 +428,8 @@ private: AtomicWord _timesOrPeriod{0}; BSONObj _data; + const std::string _name; + // protects _mode, _timesOrPeriod, _data mutable Mutex _modMutex = MONGO_MAKE_LATCH("FailPoint::_modMutex"); }; @@ -439,12 +446,12 @@ public: * 51006 - if the given name already exists in this registry. * CannotMutateObject - if this registry is already frozen. */ - Status add(const std::string& name, FailPoint* failPoint); + Status add(FailPoint* failPoint); /** * @return a registered FailPoint, or nullptr if it was not registered. */ - FailPoint* find(const std::string& name) const; + FailPoint* find(StringData name) const; /** * Freezes this registry from being modified. @@ -460,7 +467,7 @@ public: private: bool _frozen; - stdx::unordered_map _fpMap; + StringMap _fpMap; }; /** @@ -468,10 +475,15 @@ private: */ class FailPointEnableBlock { public: - explicit FailPointEnableBlock(std::string failPointName); - FailPointEnableBlock(std::string failPointName, BSONObj data); + explicit FailPointEnableBlock(StringData failPointName); + FailPointEnableBlock(StringData failPointName, BSONObj data); + explicit FailPointEnableBlock(FailPoint* failPoint); + FailPointEnableBlock(FailPoint* failPoint, BSONObj data); ~FailPointEnableBlock(); + FailPointEnableBlock(const FailPointEnableBlock&) = delete; + FailPointEnableBlock& operator=(const FailPointEnableBlock&) = delete; + // Const access to the underlying FailPoint const FailPoint* failPoint() const { return _failPoint; @@ -488,8 +500,7 @@ public: } private: - std::string _failPointName; - FailPoint* _failPoint; + FailPoint* const _failPoint; FailPoint::EntryCountT _initialTimesEntered; }; @@ -507,7 +518,7 @@ FailPoint::EntryCountT setGlobalFailPoint(const std::string& failPointName, cons */ class FailPointRegisterer { public: - FailPointRegisterer(const std::string& name, FailPoint* fp); + explicit FailPointRegisterer(FailPoint* fp); }; FailPointRegistry& globalFailPointRegistry(); @@ -518,8 +529,8 @@ FailPointRegistry& globalFailPointRegistry(); * Never use in header files, only .cpp files. */ #define MONGO_FAIL_POINT_DEFINE(fp) \ - ::mongo::FailPoint fp; \ - ::mongo::FailPointRegisterer fp##failPointRegisterer(#fp, &fp); + ::mongo::FailPoint fp(#fp); \ + ::mongo::FailPointRegisterer fp##failPointRegisterer(&fp); } // namespace mongo diff --git a/src/mongo/util/fail_point_test.cpp b/src/mongo/util/fail_point_test.cpp index 26b051fb7dc..a3c346594c9 100644 --- a/src/mongo/util/fail_point_test.cpp +++ b/src/mongo/util/fail_point_test.cpp @@ -55,12 +55,12 @@ namespace stdx = mongo::stdx; namespace mongo_test { TEST(FailPoint, InitialState) { - FailPoint failPoint; + FailPoint failPoint("testFP"); ASSERT_FALSE(failPoint.shouldFail()); } TEST(FailPoint, AlwaysOn) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::alwaysOn); ASSERT(failPoint.shouldFail()); @@ -74,7 +74,7 @@ TEST(FailPoint, AlwaysOn) { } TEST(FailPoint, NTimes) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::nTimes, 4); ASSERT(failPoint.shouldFail()); ASSERT(failPoint.shouldFail()); @@ -87,14 +87,14 @@ TEST(FailPoint, NTimes) { } TEST(FailPoint, BlockOff) { - FailPoint failPoint; + FailPoint failPoint("testFP"); bool called = false; failPoint.execute([&](const BSONObj&) { called = true; }); ASSERT_FALSE(called); } TEST(FailPoint, BlockAlwaysOn) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::alwaysOn); bool called = false; @@ -104,7 +104,7 @@ TEST(FailPoint, BlockAlwaysOn) { } TEST(FailPoint, BlockNTimes) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::nTimes, 1); size_t counter = 0; @@ -116,7 +116,7 @@ TEST(FailPoint, BlockNTimes) { } TEST(FailPoint, BlockWithException) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::alwaysOn); bool threw = false; @@ -134,7 +134,7 @@ TEST(FailPoint, BlockWithException) { } TEST(FailPoint, SetGetParam) { - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::alwaysOn, 0, BSON("x" << 20)); failPoint.execute([&](const BSONObj& data) { ASSERT_EQUALS(20, data["x"].numberInt()); }); @@ -143,12 +143,13 @@ TEST(FailPoint, SetGetParam) { class FailPointStress : public mongo::unittest::Test { public: void setUp() { - _fp.setMode(FailPoint::alwaysOn, 0, BSON("a" << 44)); + _fp = std::make_unique("testFP"); + _fp->setMode(FailPoint::alwaysOn, 0, BSON("a" << 44)); } void tearDown() { // Note: This can loop indefinitely if reference counter was off - _fp.setMode(FailPoint::off, 0, BSON("a" << 66)); + _fp->setMode(FailPoint::off, 0, BSON("a" << 66)); } void startTest() { @@ -174,7 +175,7 @@ public: private: void blockTask() { while (true) { - _fp.execute([](const BSONObj& data) { + _fp->execute([](const BSONObj& data) { // Expanded ASSERT_EQUALS since the error is not being // printed out properly if (data["a"].numberInt() != 44) { @@ -196,7 +197,7 @@ private: void blockWithExceptionTask() { while (true) { try { - _fp.execute([](const BSONObj& data) { + _fp->execute([](const BSONObj& data) { if (data["a"].numberInt() != 44) { using namespace mongo::literals; LOGV2_ERROR(24130, @@ -219,7 +220,7 @@ private: void simpleTask() { while (true) { - static_cast(MONGO_unlikely(_fp.shouldFail())); + static_cast(MONGO_unlikely(_fp->shouldFail())); stdx::lock_guard lk(_mutex); if (_inShutdown) break; @@ -228,10 +229,10 @@ private: void flipTask() { while (true) { - if (_fp.shouldFail()) { - _fp.setMode(FailPoint::off, 0); + if (_fp->shouldFail()) { + _fp->setMode(FailPoint::off, 0); } else { - _fp.setMode(FailPoint::alwaysOn, 0, BSON("a" << 44)); + _fp->setMode(FailPoint::alwaysOn, 0, BSON("a" << 44)); } stdx::lock_guard lk(_mutex); @@ -240,7 +241,7 @@ private: } } - FailPoint _fp; + std::unique_ptr _fp; std::vector _tasks; mongo::Mutex _mutex = MONGO_MAKE_LATCH(); @@ -249,7 +250,7 @@ private: TEST_F(FailPointStress, Basic) { startTest(); - mongo::sleepsecs(30); + mongo::sleepsecs(5); stopTest(); } @@ -277,7 +278,7 @@ static int64_t runParallelFailPointTest(FailPoint::Mode fpMode, const int32_t numEncountersPerThread) { ASSERT_GT(numThreads, 0); ASSERT_GT(numEncountersPerThread, 0); - FailPoint failPoint; + FailPoint failPoint("testFP"); failPoint.setMode(fpMode, fpVal); std::vector tasks; std::vector counts(numThreads, 0); @@ -398,7 +399,7 @@ TEST(FailPoint, parseBSONValidDataSucceeds) { ASSERT_TRUE(swTuple.isOK()); } -TEST(FailPoint, FailPointBlockBasicTest) { +TEST(FailPoint, FailPointEnableBlockBasicTest) { auto failPoint = mongo::globalFailPointRegistry().find("dummy"); ASSERT_FALSE(failPoint->shouldFail()); @@ -411,8 +412,21 @@ TEST(FailPoint, FailPointBlockBasicTest) { ASSERT_FALSE(failPoint->shouldFail()); } -TEST(FailPoint, FailPointBlockIfBasicTest) { - FailPoint failPoint; +TEST(FailPoint, FailPointEnableBlockByPointer) { + auto failPoint = mongo::globalFailPointRegistry().find("dummy"); + + ASSERT_FALSE(failPoint->shouldFail()); + + { + FailPointEnableBlock dummyFp(failPoint); + ASSERT_TRUE(failPoint->shouldFail()); + } + + ASSERT_FALSE(failPoint->shouldFail()); +} + +TEST(FailPoint, ExecuteIfBasicTest) { + FailPoint failPoint("testFP"); failPoint.setMode(FailPoint::nTimes, 1, BSON("skip" << true)); { bool hit = false; @@ -463,7 +477,7 @@ void assertFunctionInterruptable(std::function