Merge pull request #4117 from vespa-engine/toregge/propagate-bucket-space-to-resolved-operation-targets

Propagate bucket space to resolved operation targets.

6 files changed, 26 insertions, 23 deletions

diff --git a/storage/src/tests/distributor/operationtargetresolvertest.cpp b/storage/src/tests/distributor/operationtargetresolvertest.cpp
index 83e004f59fe..30891d17b3f 100644
--- a/storage/src/tests/distributor/operationtargetresolvertest.cpp
+++ b/storage/src/tests/distributor/operationtargetresolvertest.cpp
@@ -3,6 +3,8 @@
 #include <vespa/config/helper/configgetter.h>
 #include <vespa/document/config/config-documenttypes.h>
 #include <vespa/document/repo/documenttyperepo.h>
+#include <vespa/document/test/make_bucket_space.h>
+#include <vespa/document/test/make_document_bucket.h>
 #include <vespa/storageapi/message/bucket.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <tests/distributor/distributortestutil.h>
@@ -13,6 +15,8 @@
 #include <vespa/config/helper/configgetter.hpp>
 
 using document::BucketId;
+using document::test::makeBucketSpace;
+using document::test::makeDocumentBucket;
 
 namespace storage {
 namespace distributor {
@@ -112,19 +116,19 @@ namespace {
             BucketInstanceList result(_test.getInstances(_id, true));
             BucketInstanceList all(_test.getInstances(_id, false));
             _asserter.assertEqualMsg(
-                    all.toString(), _expected, result.createTargets());
+                    all.toString(), _expected, result.createTargets(makeBucketSpace()));
             delete _asserters.back();
             _asserters.pop_back();
         }
         
         TestTargets& sendsTo(const BucketId& id, uint16_t node) {
             _expected.push_back(OperationTarget(
-                    id, lib::Node(lib::NodeType::STORAGE, node), false));
+                    makeDocumentBucket(id), lib::Node(lib::NodeType::STORAGE, node), false));
             return *this;
         }
         TestTargets& createsAt(const BucketId& id, uint16_t node) {
             _expected.push_back(OperationTarget(
-                    id, lib::Node(lib::NodeType::STORAGE, node), true));
+                    makeDocumentBucket(id), lib::Node(lib::NodeType::STORAGE, node), true));
             return *this;
         }
 
@@ -148,7 +152,8 @@ OperationTargetResolverTest::getInstances(const BucketId& id,
     idealNodeCalc.setClusterState(getExternalOperationHandler().getClusterState());
     OperationTargetResolverImpl resolver(
             getExternalOperationHandler().getBucketDatabase(), idealNodeCalc, 16,
-            getExternalOperationHandler().getDistribution().getRedundancy());
+            getExternalOperationHandler().getDistribution().getRedundancy(),
+            makeBucketSpace());
     if (stripToRedundancy) {
         return resolver.getInstances(OperationTargetResolver::PUT, id);
     } else {
diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
index 7ef03cb696a..659a7f1d435 100644
--- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
@@ -283,7 +283,8 @@ PutOperation::onStart(DistributorMessageSender& sender)
                 _bucketSpace.getBucketDatabase(),
                 idealNodeCalculator,
                 _manager.getDistributor().getConfig().getMinimalBucketSplit(),
-                _bucketSpace.getDistribution().getRedundancy());
+                _bucketSpace.getDistribution().getRedundancy(),
+                _msg->getBucket().getBucketSpace());
         OperationTargetList targets(targetResolver.getTargets(
                 OperationTargetResolver::PUT, bid));
 
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp
index f537c6b67b6..fa4697f7861 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp
+++ b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp
@@ -6,15 +6,9 @@
 namespace storage {
 namespace distributor {
 
-document::Bucket
-OperationTarget::getBucket() const
-{
-    return document::Bucket(document::BucketSpace::placeHolder(), _bucket);
-}
-
 void
 OperationTarget::print(vespalib::asciistream& out, const PrintProperties&) const {
-    out << "OperationTarget(" << _bucket << ", " << _node
+    out << "OperationTarget(" << _bucket.toString() << ", " << _node
         << (_newCopy ? ", new copy" : ", existing copy") << ")";
 }
 
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.h b/storage/src/vespa/storage/distributor/operationtargetresolver.h
index b9f7537b5f5..23e0fbbcba4 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolver.h
+++ b/storage/src/vespa/storage/distributor/operationtargetresolver.h
@@ -15,17 +15,17 @@ namespace distributor {
 
 class OperationTarget : public vespalib::AsciiPrintable
 {
-    document::BucketId _bucket;
+    document::Bucket _bucket;
     lib::Node _node;
     bool _newCopy;
 
 public:
     OperationTarget() : _newCopy(true) {} 
-    OperationTarget(const document::BucketId& id, const lib::Node& node, bool newCopy)
-        : _bucket(id), _node(node), _newCopy(newCopy) {}
+    OperationTarget(const document::Bucket& bucket, const lib::Node& node, bool newCopy)
+        : _bucket(bucket), _node(node), _newCopy(newCopy) {}
 
-    const document::BucketId& getBucketId() const { return _bucket; }
-    document::Bucket getBucket() const;
+    document::BucketId getBucketId() const { return _bucket.getBucketId(); }
+    document::Bucket getBucket() const { return _bucket; }
     const lib::Node& getNode() const { return _node; }
     bool isNewCopy() const { return _newCopy; }
 
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
index 942ec3705bf..23bb6b1db78 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
+++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
@@ -129,12 +129,12 @@ BucketInstanceList::extendToEnoughCopies(
 }
 
 OperationTargetList
-BucketInstanceList::createTargets()
+BucketInstanceList::createTargets(document::BucketSpace bucketSpace)
 {
     OperationTargetList result;
     for (uint32_t i=0; i<_instances.size(); ++i) {
         BucketInstance& bi(_instances[i]);
-        result.push_back(OperationTarget(bi._bucket, bi._node, !bi._exist));
+        result.push_back(OperationTarget(document::Bucket(bucketSpace, bi._bucket), bi._node, !bi._exist));
     }
     return result;
 }
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
index 2f4a3e0117a..73a2c281b18 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
+++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
@@ -71,7 +71,7 @@ public:
         std::sort(_instances.begin(), _instances.end(), order);
     }
 
-    OperationTargetList createTargets();
+    OperationTargetList createTargets(document::BucketSpace bucketSpace);
 
     void print(vespalib::asciistream& out, const PrintProperties& p) const override;
 };
@@ -81,16 +81,19 @@ class OperationTargetResolverImpl : public OperationTargetResolver {
     const lib::IdealNodeCalculator& _idealNodeCalculator;
     uint32_t _minUsedBucketBits;
     uint16_t _redundancy;
+    document::BucketSpace _bucketSpace;
 
 public:
     OperationTargetResolverImpl(BucketDatabase& bucketDatabase,
                                 const lib::IdealNodeCalculator& idealNodeCalc,
                                 uint32_t minUsedBucketBits,
-                                uint16_t redundancy)
+                                uint16_t redundancy,
+                                document::BucketSpace bucketSpace)
         : _bucketDatabase(bucketDatabase),
           _idealNodeCalculator(idealNodeCalc),
           _minUsedBucketBits(minUsedBucketBits),
-          _redundancy(redundancy)
+          _redundancy(redundancy),
+          _bucketSpace(bucketSpace)
     {}
 
     BucketInstanceList getAllInstances(OperationType type,
@@ -102,7 +105,7 @@ public:
     }
 
     OperationTargetList getTargets(OperationType type, const document::BucketId& id) override {
-        return getInstances(type, id).createTargets();
+        return getInstances(type, id).createTargets(_bucketSpace);
     }
 };