diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-10-30 10:52:06 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2023-10-30 10:52:06 +0000 |
commit | d5821ee392c7a71e4db6de446b4d6adf636552e7 (patch) | |
tree | 6a5e9f8d8db00615601eed30f5a1053cbf1c15ec /searchcore/src | |
parent | 0c47a4ae1132a86c5973d8e385c793f4cf4eb7dc (diff) |
Test that documents failing move are detected and causes retry and eventual completition.
Diffstat (limited to 'searchcore/src')
5 files changed, 77 insertions, 31 deletions
diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp index ff44d898997..5c4cfb393a4 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp @@ -21,7 +21,9 @@ MyBucketModifiedHandler::notifyBucketModified(const BucketId &bucket) { MyMoveHandler::MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext) : _bucketDb(bucketDb), _moves(), - _numCachedBuckets(), + _lids2Fail(), + _numFailedMoves(0), + _numCachedBuckets(0), _storeMoveDoneContexts(storeMoveDoneContext), _moveDoneContexts() {} @@ -30,6 +32,10 @@ MyMoveHandler::~MyMoveHandler() = default; IDocumentMoveHandler::MoveResult MyMoveHandler::handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) { + if (_lids2Fail.contains(op.getPrevLid())) { + _numFailedMoves++; + return MoveResult::FAILURE; + } _moves.push_back(op); if (_bucketDb.takeGuard()->isCachedBucket(op.getBucketId())) { ++_numCachedBuckets; @@ -71,6 +77,13 @@ MySubDb::insertDocs(const UserDocuments &docs_) { } bool +MySubDb::remove(uint32_t subDbId, uint32_t lid) { + if (_subDb.sub_db_id() != subDbId) return false; + if (!_metaStore.validLid(lid)) return false; + return _metaStore.remove(lid, 0u); +} + +bool assertEqual(const document::BucketId &bucket, const proton::test::Document &doc, uint32_t sourceSubDbId, uint32_t targetSubDbId, const MoveOperation &op) { if (!EXPECT_EQUAL(bucket, op.getBucketId())) return false; diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h index ded00589f60..e21f084ff6e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h @@ -37,14 +37,25 @@ struct MyMoveHandler : public IDocumentMoveHandler { using MoveOperationVector = std::vector<MoveOperation>; bucketdb::BucketDBOwner &_bucketDb; MoveOperationVector _moves; + std::set<uint32_t> _lids2Fail; + size_t _numFailedMoves; size_t _numCachedBuckets; - bool _storeMoveDoneContexts; + bool _storeMoveDoneContexts; + std::vector<vespalib::IDestructorCallback::SP> _moveDoneContexts; - MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext = false); + explicit MyMoveHandler(bucketdb::BucketDBOwner &bucketDb) : MyMoveHandler(bucketDb, false){} + MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext); ~MyMoveHandler() override; MoveResult handleMove(MoveOperation &op, vespalib::IDestructorCallback::SP moveDoneCtx) override; + void addLid2Fail(uint32_t lid) { + _lids2Fail.insert(lid); + } + void removeLids2Fail(uint32_t lid) { + _lids2Fail.erase(lid); + } + void reset() { _moves.clear(); _numCachedBuckets = 0; @@ -66,7 +77,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { DocumentVector _docs; uint32_t _lid2Fail; - MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) + explicit MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) : _repo(std::move(repo)), _docs(), _lid2Fail(0) @@ -78,7 +89,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { void getBucketMetaData(const storage::spi::Bucket &, DocumentMetaData::Vector &) const override {} - DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return DocumentMetaData(); } + DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return {}; } Document::UP getFullDocument(DocumentIdT lid) const override { return (lid != _lid2Fail) ? Document::UP(_docs[lid]->clone()) : Document::UP(); @@ -130,11 +141,13 @@ struct MySubDb { return _docs.getBucket(userId); } - DocumentVector docs(uint32_t userId) { + DocumentVector docs(uint32_t userId) const { return _docs.getGidOrderDocs(userId); } void setBucketState(const BucketId &bucketId, bool active); + // Will remove from metastore so it is invisible. + bool remove(uint32_t subDbId, uint32_t lid); }; struct MyCountJobRunner : public IMaintenanceJobRunner { diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index 3f4fd0c1a8f..e9407d5445e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -75,6 +75,9 @@ struct ControllerFixtureBase : public ::testing::Test _bucketHandler.notifyBucketStateChanged(bucket, BucketInfo::ActiveState::NOT_ACTIVE); return *this; } + bool removeFromSubDB(uint32_t subdbId, uint32_t lid) { + return _ready.remove(subdbId, lid) || _notReady.remove(subdbId, lid); + } void failRetrieveForLid(uint32_t lid) { _ready.failRetrieveForLid(lid); _notReady.failRetrieveForLid(lid); @@ -237,6 +240,7 @@ TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_error) }); sync(); EXPECT_FALSE(_bmj->done()); + EXPECT_TRUE(docsMoved().empty()); fixRetriever(); masterExecute([this]() { EXPECT_TRUE(_bmj->scanAndMove(4, 3)); @@ -250,6 +254,38 @@ TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_error) EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); } +TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_handler_error) +{ + // bucket 2 should be moved + addReady(_ready.bucket(1)); + _bmj->recompute(); + _moveHandler.addLid2Fail(5); + masterExecute([this]() { + EXPECT_FALSE(_bmj->done()); + EXPECT_TRUE(_bmj->scanAndMove(4, 3)); + EXPECT_TRUE(_bmj->done()); + }); + sync(); + EXPECT_FALSE(_bmj->done()); + EXPECT_EQ(1u, _moveHandler._numFailedMoves); + EXPECT_EQ(1u, docsMoved().size()); + assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[0]); + const auto & doc = docsMoved()[0]; + EXPECT_TRUE(removeFromSubDB(doc.getPrevSubDbId(), doc.getPrevLid())); // Explicit remove as movehandler only observes move attempt. + _moveHandler.removeLids2Fail(5); + masterExecute([this]() { + EXPECT_TRUE(_bmj->scanAndMove(4, 3)); + EXPECT_TRUE(_bmj->done()); + }); + sync(); + EXPECT_EQ(2u, docsMoved().size()); + // First document is from previous move round. Handler just appends all operations. + assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[0]); + assertEqual(_ready.bucket(2), _ready.docs(2)[0], 1, 2, docsMoved()[1]); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); +} + TEST_F(ControllerFixture, require_that_we_move_buckets_in_several_steps) { diff --git a/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp b/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp index 815c9741c07..52acb059a97 100644 --- a/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp @@ -23,16 +23,14 @@ BucketHandler::addBucketStateChangedHandler(IBucketStateChangedHandler *handler) } void -BucketHandler::removeBucketStateChangedHandler(IBucketStateChangedHandler * - handler) +BucketHandler::removeBucketStateChangedHandler(IBucketStateChangedHandler * handler) { _handlers.erase(handler); } void BucketHandler::notifyBucketStateChanged(const document::BucketId &bucketId, - storage::spi::BucketInfo::ActiveState - newState) + storage::spi::BucketInfo::ActiveState newState) { for (auto &handler : _handlers) { handler->notifyBucketStateChanged(bucketId, newState); diff --git a/searchcore/src/vespa/searchcore/proton/test/buckethandler.h b/searchcore/src/vespa/searchcore/proton/test/buckethandler.h index 47dc3145f72..54f721fad86 100644 --- a/searchcore/src/vespa/searchcore/proton/test/buckethandler.h +++ b/searchcore/src/vespa/searchcore/proton/test/buckethandler.h @@ -6,11 +6,7 @@ #include <vespa/searchcore/proton/server/ibucketstatechangedhandler.h> #include <set> -namespace proton -{ - -namespace test -{ +namespace proton::test { /** * Test bucket handler that forwards bucket state change notifications @@ -21,22 +17,12 @@ class BucketHandler : public IBucketStateChangedNotifier std::set<IBucketStateChangedHandler *> _handlers; public: BucketHandler(); + ~BucketHandler() override; + void addBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; + void removeBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - virtual - ~BucketHandler(); - - virtual void - addBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - - virtual void - removeBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - - void - notifyBucketStateChanged(const document::BucketId &bucketId, - storage::spi::BucketInfo::ActiveState newState); + void notifyBucketStateChanged(const document::BucketId &bucketId, + storage::spi::BucketInfo::ActiveState newState); }; - -} // namespace test - -} // namespace proton +} |