Use document::Bucket instead of document::BucketId to notify bucket

ownership changes and trigger updates of storage bucket database.

8 files changed, 92 insertions, 89 deletions

diff --git a/storage/src/tests/persistence/bucketownershipnotifiertest.cpp b/storage/src/tests/persistence/bucketownershipnotifiertest.cpp
index 3340f7dabc5..9699e0907e3 100644
--- a/storage/src/tests/persistence/bucketownershipnotifiertest.cpp
+++ b/storage/src/tests/persistence/bucketownershipnotifiertest.cpp
@@ -4,6 +4,9 @@
 #include <tests/distributor/messagesenderstub.h>
 #include <tests/common/teststorageapp.h>
 #include <vespa/storage/persistence/bucketownershipnotifier.h>
+#include <vespa/document/test/make_document_bucket.h>
+
+using document::test::makeDocumentBucket;
 
 namespace storage {
 
@@ -35,22 +38,22 @@ public:
         return distributor == distributorIndex;
     }
     
-    document::BucketId getFirstNonOwnedBucket() {
+    document::Bucket getFirstNonOwnedBucket() {
         for (int i = 0; i < 1000; ++i) {
             if (!ownsBucket(0, document::BucketId(16, i))) {
-                return document::BucketId(16, i);
+                return makeDocumentBucket(document::BucketId(16, i));
             }
         }
-        return document::BucketId(0);
+        return makeDocumentBucket(document::BucketId(0));
     }
         
-    document::BucketId getFirstOwnedBucket() {
+    document::Bucket getFirstOwnedBucket() {
         for (int i = 0; i < 1000; ++i) {
             if (ownsBucket(0, document::BucketId(16, i))) {
-                return document::BucketId(16, i);
+                return makeDocumentBucket(document::BucketId(16, i));
             }
         }
-        return document::BucketId(0);
+        return makeDocumentBucket(document::BucketId(0));
     }
 
 
@@ -59,7 +62,7 @@ public:
     void testIgnoreIdealStateCalculationExceptions();
     void testGuardNotifyAlways();
 
-    void doTestNotification(const document::BucketId& bucket,
+    void doTestNotification(const document::Bucket &bucket,
                             const api::BucketInfo& info,
                             const std::string& wantedSend);
 };
@@ -75,7 +78,7 @@ BucketOwnershipNotifierTest::setUp()
 }
 
 void
-BucketOwnershipNotifierTest::doTestNotification(const document::BucketId& bucket,
+BucketOwnershipNotifierTest::doTestNotification(const document::Bucket &bucket,
                                                 const api::BucketInfo& info,
                                                 const std::string& wantedSend)
 {
@@ -93,12 +96,12 @@ void
 BucketOwnershipNotifierTest::testSendNotifyBucketChangeIfOwningDistributorChanged()
 {
     api::BucketInfo info(0x1, 2, 3);
-    document::BucketId bucket(getFirstNonOwnedBucket());
-    CPPUNIT_ASSERT(bucket.getRawId() != 0);
+    document::Bucket bucket(getFirstNonOwnedBucket());
+    CPPUNIT_ASSERT(bucket.getBucketId().getRawId() != 0);
 
     std::ostringstream wanted;
     wanted << "NotifyBucketChangeCommand("
-           << bucket
+           << bucket.getBucketId()
            << ", " << info
            << ") => 1";
 
@@ -109,8 +112,8 @@ void
 BucketOwnershipNotifierTest::testDoNotSendNotifyBucketChangeIfBucketOwnedByInitialSender()
 {
     api::BucketInfo info(0x1, 2, 3);
-    document::BucketId bucket(getFirstOwnedBucket());
-    CPPUNIT_ASSERT(bucket.getRawId() != 0);
+    document::Bucket bucket(getFirstOwnedBucket());
+    CPPUNIT_ASSERT(bucket.getBucketId().getRawId() != 0);
 
     doTestNotification(bucket, info, "");
 }
@@ -119,8 +122,8 @@ void
 BucketOwnershipNotifierTest::testIgnoreIdealStateCalculationExceptions()
 {
     api::BucketInfo info(0x1, 2, 3);
-    document::BucketId bucket(getFirstNonOwnedBucket());
-    CPPUNIT_ASSERT(bucket.getRawId() != 0);
+    document::Bucket bucket(getFirstNonOwnedBucket());
+    CPPUNIT_ASSERT(bucket.getBucketId().getRawId() != 0);
 
     _app->setClusterState(lib::ClusterState("distributor:0 storage:1"));
 
@@ -138,18 +141,18 @@ BucketOwnershipNotifierTest::testGuardNotifyAlways()
         NotificationGuard guard(notifier);
 
         api::BucketInfo info(0x1, 2, 3);
-        document::BucketId bucket1(getFirstOwnedBucket());
+        document::Bucket bucket1(getFirstOwnedBucket());
         guard.notifyAlways(bucket1, info);
 
-        document::BucketId bucket2(getFirstNonOwnedBucket());
+        document::Bucket bucket2(getFirstNonOwnedBucket());
         guard.notifyAlways(bucket2, info);
 
         wanted << "NotifyBucketChangeCommand("
-               << bucket1
+               << bucket1.getBucketId()
                << ", " << info
                << ") => 0,"
                << "NotifyBucketChangeCommand("
-               << bucket2
+               << bucket2.getBucketId()
                << ", " << info
                << ") => 1";
     }
diff --git a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
index c2669e4b43c..ae028dd20bd 100644
--- a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
+++ b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
@@ -16,11 +16,11 @@ namespace storage {
 
 uint16_t
 BucketOwnershipNotifier::getOwnerDistributorForBucket(
-        const document::BucketId& bucket) const
+        const document::Bucket &bucket) const
 {
     try {
         return (_component.getDistribution()->getIdealDistributorNode(
-                        *_component.getStateUpdater().getSystemState(), bucket));
+                        *_component.getStateUpdater().getSystemState(), bucket.getBucketId()));
         // If we get exceptions there aren't any distributors, so they'll have
         // to explicitly fetch all bucket info eventually anyway.
     } catch (lib::TooFewBucketBitsInUseException& e) {
@@ -41,7 +41,7 @@ BucketOwnershipNotifier::getOwnerDistributorForBucket(
 
 bool
 BucketOwnershipNotifier::distributorOwns(uint16_t distributor,
-                                         const document::BucketId& bucket) const
+                                         const document::Bucket &bucket) const
 {
     return (distributor == getOwnerDistributorForBucket(bucket));
 }
@@ -49,7 +49,7 @@ BucketOwnershipNotifier::distributorOwns(uint16_t distributor,
 void
 BucketOwnershipNotifier::sendNotifyBucketToDistributor(
         uint16_t distributorIndex,
-        const document::BucketId& bucket,
+        const document::Bucket &bucket,
         const api::BucketInfo& infoToSend)
 {
     if (!infoToSend.valid()) {
@@ -61,7 +61,7 @@ BucketOwnershipNotifier::sendNotifyBucketToDistributor(
         return;
     }
     api::NotifyBucketChangeCommand::SP notifyCmd(
-                new api::NotifyBucketChangeCommand(document::Bucket(BucketSpace::placeHolder(), bucket), infoToSend));
+                new api::NotifyBucketChangeCommand(bucket, infoToSend));
 
     notifyCmd->setAddress(api::StorageMessageAddress(
                                   _component.getClusterName(),
@@ -76,7 +76,7 @@ BucketOwnershipNotifier::sendNotifyBucketToDistributor(
 }
 
 void
-BucketOwnershipNotifier::logNotification(const document::BucketId& bucket,
+BucketOwnershipNotifier::logNotification(const document::Bucket &bucket,
                                          uint16_t sourceIndex,
                                          uint16_t currentOwnerIndex,
                                          const api::BucketInfo& newInfo)
@@ -85,7 +85,7 @@ BucketOwnershipNotifier::logNotification(const document::BucketId& bucket,
         "%s now owned by distributor %u, but reply for operation is scheduled "
         "to go to distributor %u. Sending NotifyBucketChange with %s to ensure "
         "new owner knows bucket exists",
-        bucket.toString().c_str(),
+        bucket.getBucketId().toString().c_str(),
         currentOwnerIndex,
         sourceIndex,
         newInfo.toString().c_str());
@@ -99,7 +99,7 @@ BucketOwnershipNotifier::logNotification(const document::BucketId& bucket,
 
 void
 BucketOwnershipNotifier::notifyIfOwnershipChanged(
-        const document::BucketId& bucket,
+        const document::Bucket &bucket,
         uint16_t sourceIndex,
         const api::BucketInfo& infoToSend)
 {
@@ -122,7 +122,7 @@ BucketOwnershipNotifier::notifyIfOwnershipChanged(
 
 void
 BucketOwnershipNotifier::sendNotifyBucketToCurrentOwner(
-        const document::BucketId& bucket,
+        const document::Bucket &bucket,
         const api::BucketInfo& infoToSend)
 {
     uint16_t distributor(getOwnerDistributorForBucket(bucket));
@@ -145,7 +145,7 @@ NotificationGuard::~NotificationGuard()
 }
 
 void
-NotificationGuard::notifyIfOwnershipChanged(const document::BucketId& bucket,
+NotificationGuard::notifyIfOwnershipChanged(const document::Bucket &bucket,
                                             uint16_t sourceIndex,
                                             const api::BucketInfo& infoToSend)
 {
@@ -153,7 +153,7 @@ NotificationGuard::notifyIfOwnershipChanged(const document::BucketId& bucket,
 }
 
 void
-NotificationGuard::notifyAlways(const document::BucketId& bucket,
+NotificationGuard::notifyAlways(const document::Bucket &bucket,
                                 const api::BucketInfo& infoToSend)
 {
     BucketToCheck bc(bucket, 0xffff, infoToSend);
diff --git a/storage/src/vespa/storage/persistence/bucketownershipnotifier.h b/storage/src/vespa/storage/persistence/bucketownershipnotifier.h
index a9abdd29bf7..06faa2ed583 100644
--- a/storage/src/vespa/storage/persistence/bucketownershipnotifier.h
+++ b/storage/src/vespa/storage/persistence/bucketownershipnotifier.h
@@ -2,7 +2,7 @@
 #pragma once
 
 #include <vector>
-#include <vespa/document/bucket/bucketid.h>
+#include <vespa/document/bucket/bucket.h>
 #include <vespa/vdslib/state/clusterstate.h>
 #include <vespa/storageapi/buckets/bucketinfo.h>
 #include <vespa/storage/common/messagesender.h>
@@ -22,13 +22,13 @@ public:
     {}
 
     bool distributorOwns(uint16_t distributor,
-                         const document::BucketId& bucket) const;
+                         const document::Bucket &bucket) const;
 
-    void notifyIfOwnershipChanged(const document::BucketId& bucket,
+    void notifyIfOwnershipChanged(const document::Bucket &bucket,
                                   uint16_t sourceIndex,
                                   const api::BucketInfo& infoToSend);
 
-    void sendNotifyBucketToCurrentOwner(const document::BucketId& bucket,
+    void sendNotifyBucketToCurrentOwner(const document::Bucket &bucket,
                                         const api::BucketInfo& infoToSend);
 private:
     enum IndexMeta {
@@ -36,13 +36,13 @@ private:
     };
 
     void sendNotifyBucketToDistributor(uint16_t distributorIndex,
-                                       const document::BucketId& bucket,
+                                       const document::Bucket &bucket,
                                        const api::BucketInfo& infoToSend);
 
     // Returns either index or FAILED_TO_RESOLVE
-    uint16_t getOwnerDistributorForBucket(const document::BucketId& bucket) const;
+    uint16_t getOwnerDistributorForBucket(const document::Bucket &bucket) const;
 
-    void logNotification(const document::BucketId& bucket,
+    void logNotification(const document::Bucket &bucket,
                          uint16_t sourceIndex,
                          uint16_t currentOwnerIndex,
                          const api::BucketInfo& newInfo);
@@ -56,7 +56,7 @@ class NotificationGuard
 {
     struct BucketToCheck
     {
-        BucketToCheck(const document::BucketId& _bucket,
+        BucketToCheck(const document::Bucket& _bucket,
                       uint16_t _sourceIndex,
                       const api::BucketInfo& _info)
           : bucket(_bucket),
@@ -65,7 +65,7 @@ class NotificationGuard
             alwaysSend(false)
         {}
 
-        document::BucketId bucket;
+        document::Bucket bucket;
         api::BucketInfo info;
         uint16_t sourceIndex;
         bool alwaysSend;
@@ -83,11 +83,11 @@ public:
 
     ~NotificationGuard();
 
-    void notifyIfOwnershipChanged(const document::BucketId& bucket,
+    void notifyIfOwnershipChanged(const document::Bucket &bucket,
                                   uint16_t sourceIndex,
                                   const api::BucketInfo& infoToSend);
 
-    void notifyAlways(const document::BucketId& bucket,
+    void notifyAlways(const document::Bucket &bucket,
                       const api::BucketInfo& infoToSend);
 };
 
diff --git a/storage/src/vespa/storage/persistence/diskmoveoperationhandler.cpp b/storage/src/vespa/storage/persistence/diskmoveoperationhandler.cpp
index 93c005728f4..299917138db 100644
--- a/storage/src/vespa/storage/persistence/diskmoveoperationhandler.cpp
+++ b/storage/src/vespa/storage/persistence/diskmoveoperationhandler.cpp
@@ -56,7 +56,7 @@ DiskMoveOperationHandler::handleBucketDiskMove(BucketDiskMoveCommand& cmd,
         return tracker;
     }
 
-    api::BucketInfo bInfo = _env.getBucketInfo(to, targetDisk);
+    api::BucketInfo bInfo = _env.getBucketInfo(bucket, targetDisk);
     uint32_t sourceFileSize = bInfo.getUsedFileSize();
 
     {
diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp
index 0b3b1558562..5fa8ca5c707 100644
--- a/storage/src/vespa/storage/persistence/mergehandler.cpp
+++ b/storage/src/vespa/storage/persistence/mergehandler.cpp
@@ -728,7 +728,7 @@ MergeHandler::applyDiffLocally(
                                  tmpInfo.isReady(),
                                  tmpInfo.isActive());
 
-    _env.updateBucketDatabase(bucket.getBucketId(), providerInfo);
+    _env.updateBucketDatabase(bucket.getBucket(), providerInfo);
     return providerInfo;
 }
 
diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp
index 75db3d3114d..d0255405dda 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.cpp
+++ b/storage/src/vespa/storage/persistence/persistencethread.cpp
@@ -215,23 +215,23 @@ PersistenceThread::handleRepairBucket(RepairBucketCommand& cmd)
     LOG(debug, "Repair(%s): %s",
         cmd.getBucketId().toString().c_str(),
         (cmd.verifyBody() ? "Verifying body" : "Not verifying body"));
-    api::BucketInfo before = _env.getBucketInfo(cmd.getBucketId());
+    api::BucketInfo before = _env.getBucketInfo(cmd.getBucket());
     spi::Result result =
         _spi.maintain(spi::Bucket(cmd.getBucket(),
                                   spi::PartitionId(_env._partition)),
                       cmd.verifyBody() ?
                       spi::HIGH : spi::LOW);
     if (checkForError(result, *tracker)) {
-        api::BucketInfo after = _env.getBucketInfo(cmd.getBucketId());
+        api::BucketInfo after = _env.getBucketInfo(cmd.getBucket());
 
         RepairBucketReply::UP reply(new RepairBucketReply(cmd, after));
         reply->setAltered(!(after == before));
         if (reply->bucketAltered()) {
-            notifyGuard.notifyAlways(cmd.getBucketId(), after);
+            notifyGuard.notifyAlways(cmd.getBucket(), after);
             ++_env._metrics.repairFixed;
         }
 
-        _env.updateBucketDatabase(cmd.getBucketId(), after);
+        _env.updateBucketDatabase(cmd.getBucket(), after);
         tracker->setReply(api::StorageReply::SP(reply.release()));
     }
     return tracker;
@@ -464,8 +464,8 @@ PersistenceThread::handleReadBucketInfo(ReadBucketInfo& cmd)
                                        _env._metrics.readBucketInfo,
                                        _env._component.getClock()));
 
-    _env.updateBucketDatabase(cmd.getBucketId(),
-                              _env.getBucketInfo(cmd.getBucketId()));
+    _env.updateBucketDatabase(cmd.getBucket(),
+                              _env.getBucketInfo(cmd.getBucket()));
     return tracker;
 }
 
@@ -539,14 +539,14 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
         // If we get here, we're splitting data in two.
         // (Possibly in special case where a target will be unused)
     assert(targetInfo.success());
-    document::BucketId target1(targetInfo.getTarget1());
-    document::BucketId target2(targetInfo.getTarget2());
+    document::Bucket target1(spiBucket.getBucketSpace(), targetInfo.getTarget1());
+    document::Bucket target2(spiBucket.getBucketSpace(), targetInfo.getTarget2());
 
     LOG(debug, "split(%s -> %s, %s)", cmd.getBucketId().toString().c_str(),
-        target1.toString().c_str(), target2.toString().c_str());
+        target1.getBucketId().toString().c_str(), target2.getBucketId().toString().c_str());
 
-    PersistenceUtil::LockResult lock1(_env.lockAndGetDisk(target1));
-    PersistenceUtil::LockResult lock2(_env.lockAndGetDisk(target2));
+    PersistenceUtil::LockResult lock1(_env.lockAndGetDisk(target1.getBucketId()));
+    PersistenceUtil::LockResult lock2(_env.lockAndGetDisk(target2.getBucketId()));
 
 #ifdef ENABLE_BUCKET_OPERATION_LOGGING
     {
@@ -554,19 +554,19 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
                 vespalib::make_string(
                         "split(%s -> %s, %s)",
                         cmd.getBucketId().toString().c_str(),
-                        target1.toString().c_str(),
-                        target2.toString().c_str()));
+                        target1.getBucketId().toString().c_str(),
+                        target2.getBucketId().toString().c_str()));
         LOG_BUCKET_OPERATION(cmd.getBucketId(), desc);
-        LOG_BUCKET_OPERATION(target1, desc);
+        LOG_BUCKET_OPERATION(target1.getBucketId(), desc);
         if (target2.getRawId() != 0) {
-            LOG_BUCKET_OPERATION(target2, desc);
+            LOG_BUCKET_OPERATION(target2.getBucketId(), desc);
         }
     }
 #endif
     spi::Result result = _spi.split(
             spiBucket,
-            spi::Bucket(document::Bucket(spiBucket.getBucketSpace(), target1), spi::PartitionId(lock1.disk)),
-            spi::Bucket(document::Bucket(spiBucket.getBucketSpace(), target2), spi::PartitionId(lock2.disk)), _context);
+            spi::Bucket(target1, spi::PartitionId(lock1.disk)),
+            spi::Bucket(target2, spi::PartitionId(lock2.disk)), _context);
     if (result.hasError()) {
         tracker->fail(_env.convertErrorCode(result),
                       result.getErrorMessage());
@@ -583,14 +583,14 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
                       FileStorHandler::RemapInfo> TargetInfo;
     std::vector<TargetInfo> targets;
     for (uint32_t i = 0; i < 2; i++) {
-        const document::BucketId& target(i == 0 ? target1 : target2);
+        const document::Bucket &target(i == 0 ? target1 : target2);
         uint16_t disk(i == 0 ? lock1.disk : lock2.disk);
-        assert(target.getRawId() != 0);
+        assert(target.getBucketId().getRawId() != 0);
         targets.push_back(TargetInfo(
                 _env.getBucketDatabase().get(
-                    target, "PersistenceThread::handleSplitBucket - Target",
+                    target.getBucketId(), "PersistenceThread::handleSplitBucket - Target",
                     StorBucketDatabase::CREATE_IF_NONEXISTING),
-                FileStorHandler::RemapInfo(target, disk)));
+                FileStorHandler::RemapInfo(target.getBucketId(), disk)));
         targets.back().first->setBucketInfo(
             _env.getBucketInfo(target, disk));
         targets.back().first->disk = disk;
@@ -601,9 +601,9 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
         LOG(spam, "split(%s - %u -> %s - %u, %s - %u)",
             cmd.getBucketId().toString().c_str(),
             targ1.getMetaCount() + targ2.getMetaCount(),
-            target1.toString().c_str(),
+            target1.getBucketId().toString().c_str(),
             targ1.getMetaCount(),
-            target2.toString().c_str(),
+            target2.getBucketId().toString().c_str(),
             targ2.getMetaCount());
     }
     FileStorHandler::RemapInfo source(cmd.getBucketId(), _env._partition);
@@ -611,11 +611,11 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
             source, targets[0].second, targets[1].second);
     bool ownershipChanged(
             !_bucketOwnershipNotifier->distributorOwns(
-                    cmd.getSourceIndex(), cmd.getBucketId()));
+                    cmd.getSourceIndex(), cmd.getBucket()));
     // Now release all the bucketdb locks.
     for (uint32_t i = 0; i < targets.size(); i++) {
         if (ownershipChanged) {
-            notifyGuard.notifyAlways(targets[i].second.bid,
+            notifyGuard.notifyAlways(document::Bucket(spiBucket.getBucketSpace(), targets[i].second.bid),
                                      targets[i].first->getBucketInfo());
         }
         // The entries vector has the source bucket in element zero, so indexing
@@ -649,7 +649,7 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd)
     }
     if (sourceEntry.exist()) {
         if (ownershipChanged) {
-            notifyGuard.notifyAlways(cmd.getBucketId(),
+            notifyGuard.notifyAlways(cmd.getBucket(),
                                      sourceEntry->getBucketInfo());
         }
         // Delete the old entry.
@@ -808,7 +808,7 @@ PersistenceThread::handleSetBucketState(api::SetBucketStateCommand& cmd)
                 cmd.getBucketId(), "handleSetBucketState"));
         if (entry.exist()) {
             entry->info.setActive(newState == spi::BucketInfo::ACTIVE);
-            notifyGuard.notifyIfOwnershipChanged(cmd.getBucketId(),
+            notifyGuard.notifyIfOwnershipChanged(cmd.getBucket(),
                     cmd.getSourceIndex(),
                     entry->info);
             entry.write();
@@ -852,7 +852,7 @@ PersistenceThread::handleInternalBucketJoin(InternalBucketJoinCommand& cmd)
         tracker->setReply(
                 api::StorageReply::SP(
                         new InternalBucketJoinReply(cmd,
-                                _env.getBucketInfo(cmd.getBucketId()))));
+                                _env.getBucketInfo(cmd.getBucket()))));
     }
     return tracker;
 }
@@ -862,21 +862,21 @@ PersistenceThread::handleRecheckBucketInfo(RecheckBucketInfoCommand& cmd)
 {
     MessageTracker::UP tracker(new MessageTracker(
             _env._metrics.recheckBucketInfo, _env._component.getClock()));
-    document::BucketId bid(cmd.getBucketId());
-    api::BucketInfo info(_env.getBucketInfo(bid));
+    document::Bucket bucket(cmd.getBucket());
+    api::BucketInfo info(_env.getBucketInfo(bucket));
     NotificationGuard notifyGuard(*_bucketOwnershipNotifier);
     {
         // Update bucket database
         StorBucketDatabase::WrappedEntry entry(
                 _component->getBucketDatabase().get(
-                        bid,
+                        bucket.getBucketId(),
                         "handleRecheckBucketInfo"));
 
         if (entry.exist()) {
             api::BucketInfo prevInfo(entry->getBucketInfo());
 
             if (!(prevInfo == info)) {
-                notifyGuard.notifyAlways(bid, info);
+                notifyGuard.notifyAlways(bucket, info);
                 entry->info = info;
                 entry.write();
             }
@@ -1189,7 +1189,9 @@ void PersistenceThread::processMessages(FileStorHandler::LockedMessage & lock)
 
         if (hasBucketInfo(*msg)) {
             if (tracker->getReply()->getResult().success()) {
-                _env.setBucketInfo(*tracker, bucketId);
+                document::Bucket bucket = msg->getBucket();
+                assert(bucket.getBucketId() == bucketId);
+                _env.setBucketInfo(*tracker, bucket);
             }
         }
         if (batchable) {
diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp
index bc0b11ae15d..e28281a5819 100644
--- a/storage/src/vespa/storage/persistence/persistenceutil.cpp
+++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp
@@ -88,12 +88,12 @@ PersistenceUtil::~PersistenceUtil()
 }
 
 void
-PersistenceUtil::updateBucketDatabase(const document::BucketId& id,
+PersistenceUtil::updateBucketDatabase(const document::Bucket &bucket,
                                       const api::BucketInfo& i)
 {
     // Update bucket database
     StorBucketDatabase::WrappedEntry entry(getBucketDatabase().get(
-                                                   id,
+                                                   bucket.getBucketId(),
                                                    "env::updatebucketdb"));
     if (entry.exist()) {
         api::BucketInfo info = i;
@@ -108,7 +108,7 @@ PersistenceUtil::updateBucketDatabase(const document::BucketId& id,
     } else {
         LOG(debug,
             "Bucket(%s).getBucketInfo: Bucket does not exist.",
-            id.toString().c_str());
+            bucket.getBucketId().toString().c_str());
     }
 }
 
@@ -147,26 +147,25 @@ PersistenceUtil::lockAndGetDisk(const document::BucketId& bucket,
 }
 
 void
-PersistenceUtil::setBucketInfo(MessageTracker& tracker,
-                               const document::BucketId& bucketId)
+PersistenceUtil::setBucketInfo(MessageTracker& tracker, const document::Bucket &bucket)
 {
-    api::BucketInfo info = getBucketInfo(bucketId, _partition);
+    api::BucketInfo info = getBucketInfo(bucket, _partition);
 
     static_cast<api::BucketInfoReply&>(*tracker.getReply()).
         setBucketInfo(info);
 
-    updateBucketDatabase(bucketId, info);
+    updateBucketDatabase(bucket, info);
 }
 
 api::BucketInfo
-PersistenceUtil::getBucketInfo(const document::BucketId& bId, int disk) const
+PersistenceUtil::getBucketInfo(const document::Bucket &bucket, int disk) const
 {
     if (disk == -1) {
         disk = _partition;
     }
 
     spi::BucketInfoResult response =
-        _spi.getBucketInfo(spi::Bucket(document::Bucket(document::BucketSpace::placeHolder(), bId), spi::PartitionId(disk)));
+        _spi.getBucketInfo(spi::Bucket(bucket, spi::PartitionId(disk)));
 
     return convertBucketInfo(response.getBucketInfo());
 }
diff --git a/storage/src/vespa/storage/persistence/persistenceutil.h b/storage/src/vespa/storage/persistence/persistenceutil.h
index 0bf13844b1d..c8ec6767c4b 100644
--- a/storage/src/vespa/storage/persistence/persistenceutil.h
+++ b/storage/src/vespa/storage/persistence/persistenceutil.h
@@ -88,7 +88,7 @@ struct PersistenceUtil {
     StorBucketDatabase& getBucketDatabase()
         { return _component.getBucketDatabase(); }
 
-    void updateBucketDatabase(const document::BucketId& id,
+    void updateBucketDatabase(const document::Bucket &bucket,
                               const api::BucketInfo& info);
 
     uint16_t getPreferredAvailableDisk(const document::BucketId& id) const;
@@ -107,12 +107,11 @@ struct PersistenceUtil {
             const document::BucketId& bucket,
             StorBucketDatabase::Flag flags = StorBucketDatabase::NONE);
 
-    api::BucketInfo getBucketInfo(const document::BucketId& bId, int disk = -1) const;
+    api::BucketInfo getBucketInfo(const document::Bucket &bucket, int disk = -1) const;
 
     api::BucketInfo convertBucketInfo(const spi::BucketInfo&) const;
 
-    void setBucketInfo(MessageTracker& tracker,
-                       const document::BucketId& bucketId);
+    void setBucketInfo(MessageTracker& tracker, const document::Bucket &bucket);
 
     static uint32_t convertErrorCode(const spi::Result& response);