diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-26 21:41:52 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-26 21:41:52 +0000 |
commit | 3a932777b5b29a6807d2e58fe5b6cb783e889782 (patch) | |
tree | 34c5974362dfd347a5e56aefc53552d33ed3e5e8 | |
parent | 925046a2377924cf2ea76a67bc8a4be31c1cf98e (diff) |
Some code healt by exposing what is necessary only
14 files changed, 45 insertions, 55 deletions
diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index 0cc9a92895b..cabcd33b2dd 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -10,6 +10,7 @@ #include <vespa/searchcore/proton/server/idocumentmovehandler.h> #include <vespa/searchcore/proton/server/imaintenancejobrunner.h> #include <vespa/searchcore/proton/server/maintenancedocumentsubdb.h> +#include <vespa/searchcore/proton/server/ibucketmodifiedhandler.h> #include <vespa/searchcore/proton/test/buckethandler.h> #include <vespa/searchcore/proton/test/clusterstatehandler.h> #include <vespa/searchcore/proton/test/disk_mem_usage_notifier.h> @@ -44,7 +45,6 @@ using BucketIdVector = BucketId::List; using DocumentVector = std::vector<Document::SP>; using MoveOperationVector = std::vector<MoveOperation>; using ScanItr = BucketMoveJob::ScanIterator; -using ScanPos = BucketMoveJob::ScanPosition; namespace { diff --git a/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.cpp b/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.cpp index d8b5aa7b129..ce3bd3b8e9b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.cpp @@ -4,6 +4,7 @@ #include "disk_mem_usage_state.h" #include "imaintenancejobrunner.h" #include "document_db_maintenance_config.h" +#include "move_operation_limiter.h" namespace proton { @@ -49,7 +50,7 @@ BlockableMaintenanceJob::BlockableMaintenanceJob(const vespalib::string &name, BlockableMaintenanceJob::~BlockableMaintenanceJob() { - _moveOpsLimiter->clearJob(); + dynamic_cast<MoveOperationLimiter &>(*_moveOpsLimiter).clearJob(); } bool diff --git a/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.h b/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.h index db7b8d05ca2..b0c69704ebe 100644 --- a/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/blockable_maintenance_job.h @@ -2,7 +2,7 @@ #pragma once #include "i_blockable_maintenance_job.h" -#include "move_operation_limiter.h" +#include "i_move_operation_limiter.h" #include <mutex> #include <unordered_set> @@ -11,6 +11,7 @@ namespace proton { class BlockableMaintenanceJobConfig; class DiskMemUsageState; class IMaintenanceJobRunner; +class IMoveOperationLimiter; /** * Implementation of a maintenance job that can be blocked and unblocked due to various external reasons. @@ -27,12 +28,10 @@ private: bool _blocked; IMaintenanceJobRunner *_runner; double _resourceLimitFactor; + std::shared_ptr<IMoveOperationLimiter> _moveOpsLimiter; void updateBlocked(const LockGuard &guard); - protected: - MoveOperationLimiter::SP _moveOpsLimiter; - void internalNotifyDiskMemUsage(const DiskMemUsageState &state); public: @@ -54,6 +53,7 @@ public: void unBlock(BlockedReason reason) override; bool isBlocked() const override; void registerRunner(IMaintenanceJobRunner *runner) override { _runner = runner; } + IMoveOperationLimiter & getLimiter() { return *_moveOpsLimiter; } }; diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index 8485cb6835e..cf6ea7f7787 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -6,6 +6,8 @@ #include "iclusterstatechangednotifier.h" #include "maintenancedocumentsubdb.h" #include "i_disk_mem_usage_notifier.h" +#include "ibucketmodifiedhandler.h" +#include "move_operation_limiter.h" #include <vespa/searchcore/proton/bucketdb/i_bucket_create_notifier.h> #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h> @@ -176,7 +178,7 @@ BucketMoveJob(const IBucketStateCalculator::SP &calc, _modifiedHandler(modifiedHandler), _ready(ready), _notReady(notReady), - _mover(*_moveOpsLimiter), + _mover(getLimiter()), _doneScan(false), _scanPos(), _scanPass(FIRST_SCAN_PASS), @@ -186,7 +188,7 @@ BucketMoveJob(const IBucketStateCalculator::SP &calc, _delayedBucketsFrozen(), _frozenBuckets(frozenBuckets), _bucketCreateNotifier(bucketCreateNotifier), - _delayedMover(*_moveOpsLimiter), + _delayedMover(getLimiter()), _clusterStateChangedNotifier(clusterStateChangedNotifier), _bucketStateChangedNotifier(bucketStateChangedNotifier), _diskMemUsageNotifier(diskMemUsageNotifier) @@ -283,8 +285,7 @@ BucketMoveJob::changedCalculator() } bool -BucketMoveJob::scanAndMove(size_t maxBucketsToScan, - size_t maxDocsToMove) +BucketMoveJob::scanAndMove(size_t maxBucketsToScan, size_t maxDocsToMove) { if (done()) { return true; @@ -306,8 +307,7 @@ BucketMoveJob::scanAndMove(size_t maxBucketsToScan, size_t bucketsScanned = 0; for (;;) { if (_mover.bucketDone()) { - ScanResult res = scanBuckets(maxBucketsToScan - - bucketsScanned, bucketGuard); + ScanResult res = scanBuckets(maxBucketsToScan - bucketsScanned, bucketGuard); bucketsScanned += res.first; if (res.second) { if (_scanPass == FIRST_SCAN_PASS && @@ -363,8 +363,7 @@ BucketMoveJob::notifyClusterStateChanged(const IBucketStateCalculator::SP &newCa } void -BucketMoveJob::notifyBucketStateChanged(const BucketId &bucketId, - BucketInfo::ActiveState newState) +BucketMoveJob::notifyBucketStateChanged(const BucketId &bucketId, BucketInfo::ActiveState newState) { // Called by master write thread if (newState == BucketInfo::NOT_ACTIVE) { diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h index e462b8ac7c6..26755eca7b1 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h @@ -6,8 +6,6 @@ #include "documentbucketmover.h" #include "i_disk_mem_usage_listener.h" #include "ibucketfreezelistener.h" -#include "ibucketmodifiedhandler.h" -#include "ibucketstatecalculator.h" #include "ibucketstatechangedhandler.h" #include "iclusterstatechangedhandler.h" #include "ifrozenbuckethandler.h" @@ -15,13 +13,14 @@ #include <vespa/searchcore/proton/bucketdb/i_bucket_create_listener.h> #include <set> -namespace proton -{ +namespace proton { class BlockableMaintenanceJobConfig; class IBucketStateChangedNotifier; class IClusterStateChangedNotifier; class IDiskMemUsageNotifier; +class IBucketModifiedHandler; + namespace bucketdb { class IBucketCreateNotifier; } /** @@ -36,19 +35,16 @@ class BucketMoveJob : public BlockableMaintenanceJob, public IDiskMemUsageListener { public: - struct ScanPosition - { + struct ScanPosition { document::BucketId _lastBucket; ScanPosition() : _lastBucket() { } - ScanPosition(document::BucketId lastBucket) : _lastBucket(lastBucket) { } bool validBucket() const { return _lastBucket.isSet(); } }; typedef BucketDB::ConstMapIterator BucketIterator; - class ScanIterator - { + class ScanIterator { private: BucketDBOwner::Guard _db; BucketIterator _itr; @@ -80,20 +76,20 @@ public: }; private: - typedef std::pair<size_t, bool> ScanResult; - IBucketStateCalculator::SP _calc; - IDocumentMoveHandler &_moveHandler; - IBucketModifiedHandler &_modifiedHandler; - const MaintenanceDocumentSubDB &_ready; - const MaintenanceDocumentSubDB &_notReady; - DocumentBucketMover _mover; - bool _doneScan; - ScanPosition _scanPos; - uint32_t _scanPass; - ScanPosition _endPos; - document::BucketSpace _bucketSpace; - - typedef std::set<document::BucketId> DelayedBucketSet; + using ScanResult = std::pair<size_t, bool>; + std::shared_ptr<IBucketStateCalculator> _calc; + IDocumentMoveHandler &_moveHandler; + IBucketModifiedHandler &_modifiedHandler; + const MaintenanceDocumentSubDB &_ready; + const MaintenanceDocumentSubDB &_notReady; + DocumentBucketMover _mover; + bool _doneScan; + ScanPosition _scanPos; + uint32_t _scanPass; + ScanPosition _endPos; + document::BucketSpace _bucketSpace; + + using DelayedBucketSet = std::set<document::BucketId>; // Delayed buckets that are no longer frozen or active that can be considered for moving. DelayedBucketSet _delayedBuckets; @@ -185,4 +181,3 @@ public: }; } // namespace proton - diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp index 6951125f408..1f2b0cf1a75 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp @@ -12,7 +12,6 @@ using document::BucketId; using document::Document; using document::GlobalId; -using search::DocumentIdT; using storage::spi::Timestamp; namespace proton { @@ -20,9 +19,7 @@ namespace proton { typedef IDocumentMetaStore::Iterator Iterator; bool -DocumentBucketMover::moveDocument(DocumentIdT lid, - const document::GlobalId &gid, - Timestamp timestamp) +DocumentBucketMover::moveDocument(uint32_t lid, const document::GlobalId &gid, Timestamp timestamp) { if ( _source->lidNeedsCommit(lid) ) { return false; @@ -80,13 +77,11 @@ namespace { class MoveKey { public: - DocumentIdT _lid; + uint32_t _lid; document::GlobalId _gid; Timestamp _timestamp; - MoveKey(DocumentIdT lid, - const document::GlobalId &gid, - Timestamp timestamp) + MoveKey(uint32_t lid, const document::GlobalId &gid, Timestamp timestamp) : _lid(lid), _gid(gid), _timestamp(timestamp) @@ -115,7 +110,7 @@ DocumentBucketMover::moveDocuments(size_t maxDocsToMove) typedef std::vector<MoveKey> MoveVec; MoveVec toMove; for (; itr != end && docsMoved < maxDocsToMove; ++itr) { - DocumentIdT lid = itr.getKey().get_lid(); + uint32_t lid = itr.getKey().get_lid(); const RawDocumentMetaData &metaData = _source->meta_store()->getRawMetaData(lid); if (metaData.getBucketUsedBits() != _bucket.getUsedBits()) { ++docsSkipped; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h index eded3a456d8..80e1811268b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h @@ -4,7 +4,6 @@ #include <vespa/document/bucket/bucketid.h> #include <vespa/document/base/globalid.h> -#include <vespa/searchlib/query/base.h> #include <persistence/spi/types.h> namespace proton { @@ -32,7 +31,7 @@ private: document::GlobalId _lastGid; bool _lastGidValid; - bool moveDocument(search::DocumentIdT lid, + bool moveDocument(uint32_t lid, const document::GlobalId &gid, storage::spi::Timestamp timestamp); diff --git a/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h b/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h index d9fc426b08a..cc91413826c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h @@ -11,7 +11,7 @@ namespace proton { * Interface used to limit the number of outstanding move operations a blockable maintenance job can have. */ struct IMoveOperationLimiter { - virtual ~IMoveOperationLimiter() {} + virtual ~IMoveOperationLimiter() = default; virtual std::shared_ptr<vespalib::IDestructorCallback> beginOperation() = 0; }; diff --git a/searchcore/src/vespa/searchcore/proton/server/ibucketmodifiedhandler.h b/searchcore/src/vespa/searchcore/proton/server/ibucketmodifiedhandler.h index 652c303283f..1c23c35e082 100644 --- a/searchcore/src/vespa/searchcore/proton/server/ibucketmodifiedhandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/ibucketmodifiedhandler.h @@ -10,7 +10,7 @@ class IBucketModifiedHandler { public: virtual void notifyBucketModified(const document::BucketId &bucket) = 0; - virtual ~IBucketModifiedHandler() {} + virtual ~IBucketModifiedHandler() = default; }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/imaintenancejobrunner.h b/searchcore/src/vespa/searchcore/proton/server/imaintenancejobrunner.h index 617aa69631f..f733821a47b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/imaintenancejobrunner.h +++ b/searchcore/src/vespa/searchcore/proton/server/imaintenancejobrunner.h @@ -14,7 +14,7 @@ public: * Schedule job to be run in the future. */ virtual void run() = 0; - virtual ~IMaintenanceJobRunner() { } + virtual ~IMaintenanceJobRunner() = default; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp index 7f60bbb3455..0e5f1de2ffe 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp @@ -30,7 +30,7 @@ LidSpaceCompactionJob::scanDocuments(const LidUsageStats &stats) if ( ! op ) { return false; } - vespalib::IDestructorCallback::SP context = _moveOpsLimiter->beginOperation(); + vespalib::IDestructorCallback::SP context = getLimiter().beginOperation(); _opStorer.appendOperation(*op, context); _handler.handleMove(*op, std::move(context)); if (isBlocked(BlockedReason::OUTSTANDING_OPS)) { diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp index f5441f86835..8750928775c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp @@ -27,7 +27,7 @@ CompactionJob::scanDocuments(const LidUsageStats &stats) DocumentMetaData document = getNextDocument(stats, false); if (document.valid()) { Bucket metaBucket(document::Bucket(_bucketSpace, document.bucketId)); - IDestructorCallback::SP context = _moveOpsLimiter->beginOperation(); + IDestructorCallback::SP context = getLimiter().beginOperation(); auto failed = _bucketExecutor.execute(metaBucket, makeBucketTask([this, meta=document, opsTracker=std::move(context)] (const Bucket & bucket, std::shared_ptr<IDestructorCallback> onDone) { assert(bucket.getBucketId() == meta.bucketId); using DoneContext = vespalib::KeepAlive<std::pair<IDestructorCallback::SP, IDestructorCallback::SP>>; diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h index 3a61ee85a87..d76a22bd601 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h @@ -7,6 +7,7 @@ namespace storage::spi { struct BucketExecutor;} namespace searchcorespi::index { class IThreadService; } +namespace vespalib { class IDestructorCallback; } namespace proton { class IDiskMemUsageNotifier; class IClusterStateChangedNotifier; diff --git a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h index 04440a7451d..f3389a1150e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h @@ -35,7 +35,7 @@ private: public: using SP = std::shared_ptr<MoveOperationLimiter>; MoveOperationLimiter(IBlockableMaintenanceJob *job, uint32_t maxOutstandingOps); - ~MoveOperationLimiter(); + ~MoveOperationLimiter() override; void clearJob(); bool isAboveLimit() const; std::shared_ptr<vespalib::IDestructorCallback> beginOperation() override; |