Merge pull request #16241 from vespa-engine/balder/some-code-health

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 78efdaebe16..095169b84ce 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 6e1b8ed79b5..cbf3de20b1f 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 35e02246255..21239532e66 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 01737336a58..b5c8b1b9998 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;
     bool hasPending() const;