From d5821ee392c7a71e4db6de446b4d6adf636552e7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 30 Oct 2023 10:52:06 +0000 Subject: Test that documents failing move are detected and causes retry and eventual completition. --- .../documentbucketmover/bucketmover_common.cpp | 15 ++++++++- .../documentbucketmover/bucketmover_common.h | 23 +++++++++++--- .../documentbucketmover_test.cpp | 36 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) (limited to 'searchcore/src/tests/proton') 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; @@ -70,6 +76,13 @@ MySubDb::insertDocs(const UserDocuments &docs_) { _docs.merge(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) { 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; bucketdb::BucketDBOwner &_bucketDb; MoveOperationVector _moves; + std::set _lids2Fail; + size_t _numFailedMoves; size_t _numCachedBuckets; - bool _storeMoveDoneContexts; + bool _storeMoveDoneContexts; + std::vector _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 repo) + explicit MyDocumentRetriever(std::shared_ptr 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) { -- cgit v1.2.3