Merge pull request #23649 from vespa-engine/balder/avoid-exposing-iterators

Balder/avoid exposing iterators

11 files changed, 81 insertions, 329 deletions

diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt
index 6ca815cd216..5435e9eb93e 100644
--- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt
+++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt
@@ -17,18 +17,6 @@ vespa_add_executable(searchcore_documentbucketmover_test_app TEST
 )
 vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app)
 
-vespa_add_executable(searchcore_scaniterator_test_app TEST
-    SOURCES
-    scaniterator_test.cpp
-    DEPENDS
-    searchcore_bucketmover_test
-    searchcore_server
-    searchcore_test
-    searchcore_feedoperation
-    GTest::GTest
-)
-vespa_add_test(NAME searchcore_scaniterator_test_app COMMAND searchcore_scaniterator_test_app)
-
 vespa_add_executable(searchcore_documentmover_test_app TEST
     SOURCES
     documentmover_test.cpp
diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp
deleted file mode 100644
index 4ec38ff7170..00000000000
--- a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp
+++ /dev/null
@@ -1,185 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "bucketmover_common.h"
-#include <vespa/searchcore/proton/bucketdb/bucketscaniterator.h>
-#include <vespa/vespalib/gtest/gtest.h>
-
-#include <vespa/log/log.h>
-LOG_SETUP("document_bucket_mover_test");
-
-using namespace proton;
-using namespace proton::move::test;
-using document::BucketId;
-
-using ScanItr = bucketdb::ScanIterator;
-
-struct ScanTestBase : public ::testing::Test
-{
-    test::UserDocumentsBuilder _builder;
-    std::shared_ptr<bucketdb::BucketDBOwner> _bucketDB;
-    MySubDb                    _ready;
-    MySubDb                    _notReady;
-    ScanTestBase();
-    ~ScanTestBase();
-
-    ScanItr getItr() {
-        return getItr(BucketId());
-    }
-
-    ScanItr getItr(BucketId bucket) {
-        return ScanItr(_bucketDB->takeGuard(), bucket);
-    }
-};
-
-ScanTestBase::ScanTestBase()
-    : _builder(),
-      _bucketDB(std::make_shared<bucketdb::BucketDBOwner>()),
-      _ready(_builder.getRepo(), _bucketDB, 1, SubDbType::READY),
-      _notReady(_builder.getRepo(), _bucketDB, 2, SubDbType::NOTREADY)
-{}
-ScanTestBase::~ScanTestBase() = default;
-
-struct ScanTest : public ScanTestBase
-{
-    ScanTest() : ScanTestBase()
-    {
-        _builder.createDocs(6, 1, 2);
-        _builder.createDocs(8, 2, 3);
-        _ready.insertDocs(_builder.getDocs());
-        _builder.clearDocs();
-        _builder.createDocs(2, 1, 2);
-        _builder.createDocs(4, 2, 3);
-        _notReady.insertDocs(_builder.getDocs());
-        _builder.clearDocs();
-    }
-};
-
-struct OnlyNotReadyScanTest : public ScanTestBase
-{
-    OnlyNotReadyScanTest() : ScanTestBase()
-    {
-        _builder.createDocs(2, 1, 2);
-        _builder.createDocs(4, 2, 3);
-        _notReady.insertDocs(_builder.getDocs());
-    }
-};
-
-struct OnlyReadyScanTest : public ScanTestBase
-{
-    OnlyReadyScanTest() : ScanTestBase()
-    {
-        _builder.createDocs(6, 1, 2);
-        _builder.createDocs(8, 2, 3);
-        _ready.insertDocs(_builder.getDocs());
-    }
-};
-
-struct BucketVector : public BucketId::List
-{
-    BucketVector() : BucketId::List() {}
-    BucketVector &add(const BucketId &bucket) {
-        push_back(bucket);
-        return *this;
-    }
-};
-
-void
-advanceToFirstBucketWithDocs(ScanItr &itr, SubDbType subDbType)
-{
-    while (itr.valid()) {
-        if (subDbType == SubDbType::READY) {
-            if (itr.hasReadyBucketDocs())
-                return;
-        } else {
-            if (itr.hasNotReadyBucketDocs())
-                return;
-        }
-        ++itr;
-    }
-}
-
-void
-assertEquals(const BucketVector &exp, ScanItr &itr, SubDbType subDbType)
-{
-    for (size_t i = 0; i < exp.size(); ++i) {
-        advanceToFirstBucketWithDocs(itr, subDbType);
-        EXPECT_TRUE(itr.valid());
-        EXPECT_EQ(exp[i], itr.getBucket());
-        ++itr;
-    }
-    advanceToFirstBucketWithDocs(itr, subDbType);
-    EXPECT_FALSE(itr.valid());
-}
-
-TEST_F(ScanTest, require_that_we_can_iterate_all_buckets_from_start_to_end)
-{
-    {
-        ScanItr itr = getItr();
-        assertEquals(BucketVector().
-                     add(_notReady.bucket(2)).
-                     add(_notReady.bucket(4)), itr, SubDbType::NOTREADY);
-    }
-    {
-        ScanItr itr = getItr();
-        assertEquals(BucketVector().
-                     add(_ready.bucket(6)).
-                     add(_ready.bucket(8)), itr, SubDbType::READY);
-    }
-}
-
-TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_not_ready_buckets)
-{
-    BucketId bucket = _notReady.bucket(4);
-    {
-        ScanItr itr = getItr(bucket);
-        assertEquals(BucketVector().
-                     add(_notReady.bucket(4)), itr, SubDbType::NOTREADY);
-    }
-    {
-        ScanItr itr = getItr();
-        assertEquals(BucketVector().
-                     add(_ready.bucket(6)).
-                     add(_ready.bucket(8)), itr, SubDbType::READY);
-    }
-}
-
-TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_ready_buckets)
-{
-    {
-        ScanItr itr = getItr();
-        assertEquals(BucketVector().
-                     add(_notReady.bucket(2)).
-                     add(_notReady.bucket(4)), itr, SubDbType::NOTREADY);
-    }
-    {
-        BucketId bucket = _ready.bucket(6);
-        ScanItr itr = getItr(bucket);
-        assertEquals(BucketVector().
-                     add(_ready.bucket(6)).
-                     add(_ready.bucket(8)), itr, SubDbType::READY);
-    }
-}
-
-TEST_F(OnlyNotReadyScanTest, require_that_we_can_iterate_only_not_ready_buckets)
-{
-    ScanItr itr = getItr();
-    assertEquals(BucketVector().
-                 add(_notReady.bucket(2)).
-                 add(_notReady.bucket(4)), itr, SubDbType::NOTREADY);
-}
-
-TEST_F(OnlyReadyScanTest, require_that_we_can_iterate_only_ready_buckets)
-{
-    ScanItr itr = getItr();
-    assertEquals(BucketVector().
-                 add(_ready.bucket(6)).
-                 add(_ready.bucket(8)), itr, SubDbType::READY);
-}
-
-TEST_F(ScanTestBase, require_that_we_can_iterate_zero_buckets)
-{
-    ScanItr itr = getItr();
-    EXPECT_FALSE(itr.valid());
-}
-
-GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt
index 810fe53236d..a542f44f27e 100644
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt
+++ b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt
@@ -6,7 +6,6 @@ vespa_add_library(searchcore_bucketdb STATIC
     bucket_db_owner.cpp
     bucketdb.cpp
     bucketdbhandler.cpp
-    bucketscaniterator.cpp
     bucketsessionbase.cpp
     bucketstate.cpp
     checksumaggregators.cpp
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp
index b68892be60f..9d28d88ba4a 100644
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp
@@ -33,10 +33,11 @@ checksumToString(storage::spi::BucketChecksum checksum)
 void
 convertBucketsToSlime(const BucketDB &bucketDb, Cursor &array)
 {
-    for (auto itr = bucketDb.begin(); itr != bucketDb.end(); ++itr) {
+    BucketId::List buckets = bucketDb.getBuckets();
+    for (BucketId bucketId : buckets) {
         Cursor &object = array.addObject();
-        object.setString("id", bucketIdToString(itr->first));
-        const bucketdb::BucketState &state = itr->second;
+        object.setString("id", bucketIdToString(bucketId));
+        bucketdb::BucketState state = bucketDb.get(bucketId);
         object.setString("checksum", checksumToString(state.getChecksum()));
         object.setLong("readyCount", state.getReadyCount());
         object.setLong("notReadyCount", state.getNotReadyCount());
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp
index ecdc679e4bc..a99d5cbd573 100644
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp
@@ -2,6 +2,7 @@
 
 #include "bucketdb.h"
 #include "remove_batch_entry.h"
+#include <vespa/vespalib/stllike/hash_map.hpp>
 #include <cassert>
 #include <algorithm>
 #include <optional>
@@ -27,22 +28,19 @@ BucketDB::~BucketDB()
 }
 
 void
-BucketDB::add(const BucketId &bucketId, const BucketState & state) {
+BucketDB::add(BucketId bucketId, const BucketState & state) {
     _map[bucketId] += state;
 }
 
 bucketdb::BucketState *
-BucketDB::getBucketStatePtr(const BucketId &bucket)
+BucketDB::getBucketStatePtr(BucketId bucket)
 {
     auto it(_map.find(bucket));
-    if (it != _map.end()) {
-        return &it->second;
-    }
-    return nullptr;
+    return (it != _map.end()) ? &it->second : nullptr;
 }
 
 void
-BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta)
+BucketDB::unloadBucket(BucketId bucket, const BucketState &delta)
 {
     BucketState *state = getBucketStatePtr(bucket);
     assert(state);
@@ -51,7 +49,7 @@ BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta)
 
 const bucketdb::BucketState &
 BucketDB::add(const GlobalId &gid,
-              const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize,
+              BucketId bucketId, Timestamp timestamp, uint32_t docSize,
               SubDbType subDbType)
 {
     BucketState &state = _map[bucketId];
@@ -61,7 +59,7 @@ BucketDB::add(const GlobalId &gid,
 
 void
 BucketDB::remove(const GlobalId &gid,
-                 const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize,
+                 BucketId bucketId, Timestamp timestamp, uint32_t docSize,
                  SubDbType subDbType)
 {
     BucketState &state = _map[bucketId];
@@ -84,8 +82,8 @@ BucketDB::remove_batch(const std::vector<RemoveBatchEntry> &removed, SubDbType s
 
 void
 BucketDB::modify(const GlobalId &gid,
-                 const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize,
-                 const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize,
+                 BucketId oldBucketId, Timestamp oldTimestamp, uint32_t oldDocSize,
+                 BucketId newBucketId, Timestamp newTimestamp, uint32_t newDocSize,
                  SubDbType subDbType)
 {
     if (oldBucketId == newBucketId) {
@@ -99,17 +97,14 @@ BucketDB::modify(const GlobalId &gid,
 
 
 bucketdb::BucketState
-BucketDB::get(const BucketId &bucketId) const
+BucketDB::get(BucketId bucketId) const
 {
     auto itr = _map.find(bucketId);
-    if (itr != _map.end()) {
-        return itr->second;
-    }
-    return BucketState();
+    return (itr != _map.end()) ? itr->second : BucketState();
 }
 
 void
-BucketDB::cacheBucket(const BucketId &bucketId)
+BucketDB::cacheBucket(BucketId bucketId)
 {
     _cachedBucketId = bucketId;
     _cachedBucketState = get(bucketId);
@@ -123,13 +118,13 @@ BucketDB::uncacheBucket()
 }
 
 bool
-BucketDB::isCachedBucket(const BucketId &bucketId) const
+BucketDB::isCachedBucket(BucketId bucketId) const
 {
     return _cachedBucketId == bucketId;
 }
 
 bucketdb::BucketState
-BucketDB::cachedGet(const BucketId &bucketId) const
+BucketDB::cachedGet(BucketId bucketId) const
 {
     if (isCachedBucket(bucketId)) {
         return _cachedBucketState;
@@ -138,27 +133,23 @@ BucketDB::cachedGet(const BucketId &bucketId) const
 }
 
 storage::spi::BucketInfo
-BucketDB::cachedGetBucketInfo(const BucketId &bucketId) const
+BucketDB::cachedGetBucketInfo(BucketId bucketId) const
 {
     if (isCachedBucket(bucketId)) {
         return _cachedBucketState;
     }
-    auto itr = _map.find(bucketId);
-    if (itr != _map.end()) {
-        return itr->second;
-    }
-    return BucketState();
+    return get(bucketId);
 }
 
 bool
-BucketDB::hasBucket(const BucketId &bucketId) const
+BucketDB::hasBucket(BucketId bucketId) const
 {
     return (_map.find(bucketId) != _map.end());
 }
 
 
 bool
-BucketDB::isActiveBucket(const BucketId &bucketId) const
+BucketDB::isActiveBucket(BucketId bucketId) const
 {
     auto itr = _map.find(bucketId);
     return (itr != _map.end()) && itr->second.isActive();
@@ -172,6 +163,7 @@ BucketDB::getBuckets() const
     for (const auto & entry : _map) {
         buckets.push_back(entry.first);
     }
+    std::sort(buckets.begin(), buckets.end());
     return buckets;
 }
 
@@ -199,7 +191,7 @@ BucketDB::checkEmpty() const
 
 
 void
-BucketDB::setBucketState(const BucketId &bucketId, bool active)
+BucketDB::setBucketState(BucketId bucketId, bool active)
 {
     BucketState &state = _map[bucketId];
     state.setActive(active);
@@ -207,7 +199,7 @@ BucketDB::setBucketState(const BucketId &bucketId, bool active)
 
 
 void
-BucketDB::createBucket(const BucketId &bucketId)
+BucketDB::createBucket(BucketId bucketId)
 {
     BucketState &state = _map[bucketId];
     (void) state;
@@ -215,7 +207,7 @@ BucketDB::createBucket(const BucketId &bucketId)
 
 
 void
-BucketDB::deleteEmptyBucket(const BucketId &bucketId)
+BucketDB::deleteEmptyBucket(BucketId bucketId)
 {
     auto itr = _map.find(bucketId);
     if (itr == _map.end()) {
@@ -231,11 +223,13 @@ document::BucketId::List
 BucketDB::getActiveBuckets() const
 {
     BucketId::List buckets;
+    buckets.reserve(_map.size());
     for (const auto & entry : _map) {
         if (entry.second.isActive()) {
             buckets.push_back(entry.first);
         }
     }
+    std::sort(buckets.begin(), buckets.end());
     return buckets;
 }
 
@@ -247,11 +241,12 @@ BucketDB::populateActiveBuckets(BucketId::List buckets)
     std::sort(buckets.begin(), buckets.end());
     auto si = buckets.begin();
     auto se = buckets.end();
-    for (const auto & entry : _map) {
-        for (; si != se && !(entry.first < *si); ++si) {
-            if (*si < entry.first) {
+    BucketId::List currentBuckets = getBuckets();
+    for (BucketId bucketId : currentBuckets) {
+        for (; si != se && !(bucketId < *si); ++si) {
+            if (*si < bucketId) {
                 toAdd.push_back(*si);
-            } else if (!entry.second.isActive()) {
+            } else if (!isActiveBucket(bucketId)) {
                 fixupBuckets.push_back(*si);
                 setBucketState(*si, true);
             }
@@ -262,8 +257,8 @@ BucketDB::populateActiveBuckets(BucketId::List buckets)
     }
     BucketState activeState;
     activeState.setActive(true);
-    for (const BucketId & bucketId : toAdd) {
-        auto [itr, inserted] = _map.emplace(bucketId, activeState);
+    for (BucketId  bucketId : toAdd) {
+        auto [itr, inserted] = _map.insert(std::make_pair(bucketId, activeState));
         assert(inserted);
     }
     return fixupBuckets;
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h
index da475f2969a..bd6aa16504b 100644
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h
+++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h
@@ -5,7 +5,7 @@
 #include "bucketstate.h"
 #include <vespa/document/bucket/bucketid.h>
 #include <vespa/persistence/spi/result.h>
-#include <map>
+#include <vespa/vespalib/stllike/hash_map.h>
 
 namespace proton::bucketdb { class RemoveBatchEntry; }
 
@@ -19,8 +19,7 @@ public:
     typedef storage::spi::Timestamp Timestamp;
     typedef storage::spi::BucketChecksum BucketChecksum;
     typedef bucketdb::BucketState BucketState;
-    typedef std::map<BucketId, BucketState> Map;
-    typedef Map::const_iterator ConstMapIterator;
+    typedef vespalib::hash_map<BucketId, BucketState, BucketId::hash> Map;
 
 private:
     Map _map;
@@ -34,43 +33,39 @@ public:
     ~BucketDB();
 
     const BucketState & add(const GlobalId &gid,
-                            const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize,
+                            BucketId bucketId, Timestamp  timestamp, uint32_t docSize,
                             SubDbType subDbType);
 
-    void add(const BucketId &bucketId, const BucketState & state);
+    void add(BucketId bucketId, const BucketState & state);
     void remove(const GlobalId &gid,
-                const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize,
+                BucketId bucketId, Timestamp  timestamp, uint32_t docSize,
                 SubDbType subDbType);
 
     void remove_batch(const std::vector<bucketdb::RemoveBatchEntry> &removed, SubDbType sub_db_type);
 
     void modify(const GlobalId &gid,
-                const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize,
-                const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize,
+                BucketId oldBucketId, Timestamp  oldTimestamp, uint32_t oldDocSize,
+                BucketId newBucketId, Timestamp  newTimestamp, uint32_t newDocSize,
                 SubDbType subDbType);
 
-    BucketState get(const BucketId &bucketId) const;
-    void cacheBucket(const BucketId &bucketId);
+    BucketState get(BucketId bucketId) const;
+    void cacheBucket(BucketId bucketId);
     void uncacheBucket();
-    bool isCachedBucket(const BucketId &bucketId) const;
-    storage::spi::BucketInfo cachedGetBucketInfo(const BucketId &bucketId) const;
-    BucketState cachedGet(const BucketId &bucketId) const;
-    bool hasBucket(const BucketId &bucketId) const;
+    bool isCachedBucket(BucketId bucketId) const;
+    storage::spi::BucketInfo cachedGetBucketInfo(BucketId bucketId) const;
+    BucketState cachedGet(BucketId bucketId) const;
+    bool hasBucket(BucketId bucketId) const;
     BucketId::List getBuckets() const;
     bool empty() const;
-    void setBucketState(const BucketId &bucketId, bool active);
-    void createBucket(const BucketId &bucketId);
-    void deleteEmptyBucket(const BucketId &bucketId);
+    void setBucketState(BucketId bucketId, bool active);
+    void createBucket(BucketId bucketId);
+    void deleteEmptyBucket(BucketId bucketId);
     BucketId::List getActiveBuckets() const;
     BucketId::List populateActiveBuckets(BucketId::List buckets);
-
-    ConstMapIterator begin() const { return _map.begin(); }
-    ConstMapIterator end() const { return _map.end(); }
-    ConstMapIterator lowerBound(const BucketId &bucket) const { return _map.lower_bound(bucket); }
     size_t size() const { return _map.size(); }
-    bool isActiveBucket(const BucketId &bucketId) const;
-    BucketState *getBucketStatePtr(const BucketId &bucket);
-    void unloadBucket(const BucketId &bucket, const BucketState &delta);
+    bool isActiveBucket(BucketId bucketId) const;
+    BucketState *getBucketStatePtr(BucketId bucket);
+    void unloadBucket(BucketId bucket, const BucketState &delta);
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp
deleted file mode 100644
index c691e786add..00000000000
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "bucketscaniterator.h"
-
-using document::BucketId;
-
-namespace proton::bucketdb {
-
-ScanIterator::ScanIterator(const Guard & db, BucketId bucket)
-    : _db(std::move(db)),
-      _itr(_db->lowerBound(bucket)),
-      _end(_db->end())
-{ }
-
-} // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h
deleted file mode 100644
index 1b68cfc1e59..00000000000
--- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include "bucket_db_owner.h"
-#include "bucketdb.h"
-
-namespace proton::bucketdb {
-
-
-class ScanIterator {
-private:
-    using BucketId = document::BucketId;
-    using BucketIterator = BucketDB::ConstMapIterator;
-    const Guard         &_db;
-    BucketIterator       _itr;
-    BucketIterator       _end;
-
-public:
-    ScanIterator(const Guard & db, BucketId bucket);
-    ScanIterator(const ScanIterator &) = delete;
-    ScanIterator(ScanIterator &&rhs) = delete;
-    ScanIterator &operator=(const ScanIterator &) = delete;
-    ScanIterator &operator=(ScanIterator &&rhs) = delete;
-
-    bool                   valid() const { return _itr != _end; }
-    bool                isActive() const { return _itr->second.isActive(); }
-    BucketId           getBucket() const { return _itr->first; }
-    bool      hasReadyBucketDocs() const { return _itr->second.getReadyCount() != 0; }
-    bool   hasNotReadyBucketDocs() const { return _itr->second.getNotReadyCount() != 0; }
-
-    ScanIterator & operator++() {
-        ++_itr;
-        return *this;
-    }
-};
-
-}
diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp
index 45cac1d02e9..415000c3f9b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp
@@ -9,6 +9,7 @@
 #include "document_db_maintenance_config.h"
 #include <vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h>
 #include <vespa/searchcore/proton/bucketdb/i_bucket_create_notifier.h>
+#include <vespa/searchcore/proton/bucketdb/bucket_db_owner.h>
 #include <vespa/searchcore/proton/feedoperation/moveoperation.h>
 #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h>
 #include <vespa/searchcorespi/index/i_thread_service.h>
@@ -141,7 +142,7 @@ BucketMoveJob::create(const std::shared_ptr<IBucketStateCalculator> &calc,
 }
 
 BucketMoveJob::NeedResult
-BucketMoveJob::needMove(const ScanIterator &itr) const {
+BucketMoveJob::needMove(BucketId bucketId, const BucketStateWrapper &itr) const {
     NeedResult noMove(false, false);
     const bool hasReadyDocs = itr.hasReadyBucketDocs();
     const bool hasNotReadyDocs = itr.hasNotReadyBucketDocs();
@@ -154,13 +155,13 @@ BucketMoveJob::needMove(const ScanIterator &itr) const {
     if (!_calc || (_calc->nodeRetired() && !isActive)) {
         return noMove;
     }
-    const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, itr.getBucket()));
+    const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, bucketId));
     if (shouldBeReady == Trinary::Undefined) {
         return noMove;
     }
     const bool wantReady = (shouldBeReady == Trinary::True) || isActive;
     LOG(spam, "needMove(): bucket(%s), shouldBeReady(%s), active(%s)",
-        itr.getBucket().toString().c_str(), toStr(shouldBeReady), toStr(isActive));
+        bucketId.toString().c_str(), toStr(shouldBeReady), toStr(isActive));
     if (wantReady) {
         if (!hasNotReadyDocs) {
             return noMove; // No notready bucket to make ready
@@ -301,8 +302,7 @@ BucketMoveJob::considerBucket(const bucketdb::Guard & guard, BucketId bucket) {
 void
 BucketMoveJob::reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket) {
     assert( ! _bucketsInFlight.contains(bucket));
-    ScanIterator itr(guard, bucket);
-    auto [mustMove, wantReady] = needMove(itr);
+    auto [mustMove, wantReady] = needMove(bucket, guard->get(bucket));
     if (mustMove) {
         _buckets2Move[bucket] = wantReady;
     } else {
@@ -322,10 +322,11 @@ BucketMoveJob::BucketMoveSet
 BucketMoveJob::computeBuckets2Move(const bucketdb::Guard & guard)
 {
     BucketMoveJob::BucketMoveSet toMove;
-    for (ScanIterator itr(guard, BucketId()); itr.valid(); ++itr) {
-        auto [mustMove, wantReady] = needMove(itr);
+    BucketId::List buckets = guard->getBuckets();
+    for (BucketId bucketId : buckets) {
+        auto [mustMove, wantReady] = needMove(bucketId, guard->get(bucketId));
         if (mustMove) {
-            toMove[itr.getBucket()] = wantReady;
+            toMove[bucketId] = wantReady;
         }
     }
     return toMove;
diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h
index d4438fcd411..9885b581a24 100644
--- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h
+++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h
@@ -8,9 +8,9 @@
 #include "ibucketstatechangedhandler.h"
 #include "iclusterstatechangedhandler.h"
 #include "maintenancedocumentsubdb.h"
-#include <vespa/searchcore/proton/bucketdb/bucketscaniterator.h>
 #include <vespa/searchcore/proton/bucketdb/i_bucket_create_listener.h>
 #include <vespa/vespalib/util/retain_guard.h>
+#include <map>
 
 
 namespace storage::spi { struct BucketExecutor; }
@@ -49,7 +49,6 @@ private:
     using IDestructorCallbackSP = std::shared_ptr<IDestructorCallback>;
     using IThreadService = searchcorespi::index::IThreadService;
     using BucketId = document::BucketId;
-    using ScanIterator = bucketdb::ScanIterator;
     using BucketMoveSet = std::map<BucketId, bool>;
     using NeedResult = std::pair<bool, bool>;
     using ActiveState = storage::spi::BucketInfo::ActiveState;
@@ -79,6 +78,17 @@ private:
     IBucketStateChangedNotifier       &_bucketStateChangedNotifier;
     IDiskMemUsageNotifier             &_diskMemUsageNotifier;
 
+    class BucketStateWrapper {
+    private:
+        const bucketdb::BucketState & _state;
+
+    public:
+        BucketStateWrapper(const bucketdb::BucketState & state) noexcept : _state(state) {}
+
+        bool                isActive() const noexcept { return _state.isActive(); }
+        bool      hasReadyBucketDocs() const noexcept { return _state.getReadyCount() != 0; }
+        bool   hasNotReadyBucketDocs() const noexcept { return _state.getNotReadyCount() != 0; }
+    };
     BucketMoveJob(const std::shared_ptr<IBucketStateCalculator> &calc,
                   vespalib::RetainGuard dbRetainer,
                   IDocumentMoveHandler &moveHandler,
@@ -103,7 +113,7 @@ private:
     void reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket);
     void updatePending();
     void cancelBucket(BucketId bucket); // True if something to cancel
-    NeedResult needMove(const ScanIterator &itr) const;
+    NeedResult needMove(BucketId bucketId, const BucketStateWrapper &itr) const;
     BucketMoveSet computeBuckets2Move(const bucketdb::Guard & guard);
     BucketMoverSP createMover(BucketId bucket, bool wantReady);
     BucketMoverSP greedyCreateMover();
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index 3fdf82ea61d..783282b71c1 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -1025,11 +1025,12 @@ notifyBucketsChanged(const documentmetastore::IBucketHandler &metaStore,
                      IBucketModifiedHandler &handler,
                      const vespalib::string &name)
 {
-    bucketdb::Guard buckets = metaStore.getBucketDB().takeGuard();
-    for (const auto &kv : *buckets) {
-        handler.notifyBucketModified(kv.first);
+    bucketdb::Guard guard = metaStore.getBucketDB().takeGuard();
+    document::BucketId::List buckets = guard->getBuckets();
+    for (document::BucketId bucketId : buckets) {
+        handler.notifyBucketModified(bucketId);
     }
-    LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets->size());
+    LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets.size());
 }
 
 }