review follow-up

* add noexcept to new APIs
* use "const auto *"
* add documentation comments
* add copyright

7 files changed, 23 insertions, 11 deletions

diff --git a/storage/src/tests/distributor/dummy_cluster_context.h b/storage/src/tests/distributor/dummy_cluster_context.h
index 1efbec4099e..783038daf7d 100644
--- a/storage/src/tests/distributor/dummy_cluster_context.h
+++ b/storage/src/tests/distributor/dummy_cluster_context.h
@@ -1,3 +1,5 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
 #pragma once
 
 #include <vespa/storage/common/cluster_context.h>
diff --git a/storage/src/vespa/storage/common/cluster_context.h b/storage/src/vespa/storage/common/cluster_context.h
index 7d6c31b1981..a7d8ab0a2e5 100644
--- a/storage/src/vespa/storage/common/cluster_context.h
+++ b/storage/src/vespa/storage/common/cluster_context.h
@@ -6,24 +6,34 @@
 
 namespace storage {
 
+/**
+ * Cluster ontext common to all storage components.
+ * For now just the cluster name, but we can consider
+ * moving other global context into this API.
+ **/
 struct ClusterContext {
 protected:
     virtual ~ClusterContext() = default;
 public:
-    // returns a pointer to the cluster name
-    // must be a valid pointer to a constant string for the
-    // lifetime of all the components that may ask for it
-    virtual const vespalib::string * cluster_name_ptr() const = 0;
+    // Returns a pointer to the cluster name.
+    // Must be a valid pointer to a constant string for the
+    // lifetime of all the components that may ask for it.
+    // This API is for the benefit of StorageMessageAddress
+    // which wants to contain the pointer returned here.
+    virtual const vespalib::string * cluster_name_ptr() const noexcept = 0;
 
     // convenience method
-    const vespalib::string &cluster_name() const {
+    const vespalib::string &cluster_name() const noexcept {
         return *cluster_name_ptr();
     }
 };
 
+/**
+ * Simple ClusterContext with an exposed string.
+ **/
 struct SimpleClusterContext : ClusterContext {
     vespalib::string my_cluster_name;
-    const vespalib::string * cluster_name_ptr() const override {
+    const vespalib::string * cluster_name_ptr() const noexcept override {
         return &my_cluster_name;
     }
     SimpleClusterContext() : my_cluster_name("") {}
diff --git a/storage/src/vespa/storage/common/storagecomponent.h b/storage/src/vespa/storage/common/storagecomponent.h
index 021a63729d7..4fda62929e7 100644
--- a/storage/src/vespa/storage/common/storagecomponent.h
+++ b/storage/src/vespa/storage/common/storagecomponent.h
@@ -82,7 +82,7 @@ public:
     StorageComponent(StorageComponentRegister&, vespalib::stringref name);
     ~StorageComponent() override;
 
-    const ClusterContext & cluster_context() const { return _cluster_ctx; }
+    const ClusterContext & cluster_context() const noexcept { return _cluster_ctx; }
     const lib::NodeType& getNodeType() const { return *_nodeType; }
     uint16_t getIndex() const { return _index; }
     lib::Node getNode() const { return lib::Node(*_nodeType, _index); }
diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.h b/storage/src/vespa/storage/distributor/distributorcomponent.h
index a62e6066a03..13c0a72d44e 100644
--- a/storage/src/vespa/storage/distributor/distributorcomponent.h
+++ b/storage/src/vespa/storage/distributor/distributorcomponent.h
@@ -147,7 +147,7 @@ public:
 
     // Implements DistributorNodeContext
     const framework::Clock& clock() const noexcept override { return getClock(); }
-    const vespalib::string * cluster_name_ptr() const override { return cluster_context().cluster_name_ptr(); }
+    const vespalib::string * cluster_name_ptr() const noexcept override { return cluster_context().cluster_name_ptr(); }
     const document::BucketIdFactory& bucket_id_factory() const noexcept override { return getBucketIdFactory(); }
     uint16_t node_index() const noexcept override { return getIndex(); }
 
diff --git a/storage/src/vespa/storage/distributor/distributormessagesender.cpp b/storage/src/vespa/storage/distributor/distributormessagesender.cpp
index e1fa7a1706e..3afd2ea8139 100644
--- a/storage/src/vespa/storage/distributor/distributormessagesender.cpp
+++ b/storage/src/vespa/storage/distributor/distributormessagesender.cpp
@@ -10,7 +10,7 @@ DistributorMessageSender::sendToNode(const lib::NodeType& nodeType, uint16_t nod
                                      const std::shared_ptr<api::StorageCommand> & cmd, bool useDocumentAPI)
 {
     cmd->setSourceIndex(getDistributorIndex());
-    auto cluster_np = cluster_context().cluster_name_ptr();
+    const auto *cluster_np = cluster_context().cluster_name_ptr();
     cmd->setAddress(useDocumentAPI
                     ? api::StorageMessageAddress::createDocApi(cluster_np, nodeType, node)
                     : api::StorageMessageAddress::create(cluster_np, nodeType, node));
diff --git a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
index 260a5559e55..4178b2bb947 100644
--- a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
+++ b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp
@@ -59,7 +59,7 @@ BucketOwnershipNotifier::sendNotifyBucketToDistributor(
     }
     auto notifyCmd = std::make_shared<api::NotifyBucketChangeCommand>(bucket, infoToSend);
 
-    auto cluster_np = _component.cluster_context().cluster_name_ptr();
+    const auto *cluster_np = _component.cluster_context().cluster_name_ptr();
     notifyCmd->setAddress(api::StorageMessageAddress::create(cluster_np, lib::NodeType::DISTRIBUTOR, distributorIndex));
     notifyCmd->setSourceIndex(_component.getIndex());
     LOG(debug,
diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp
index e6c97ea96be..2984ba27344 100644
--- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp
+++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp
@@ -373,7 +373,7 @@ MergeThrottler::forwardCommandToNode(
                     mergeCmd.getMaxTimestamp(),
                     mergeCmd.getClusterStateVersion(),
                     newChain));
-    auto cluster_np = _component.cluster_context().cluster_name_ptr();
+    const auto *cluster_np = _component.cluster_context().cluster_name_ptr();
     fwdMerge->setAddress(api::StorageMessageAddress::create(cluster_np, lib::NodeType::STORAGE, nodeIndex));
     fwdMerge->setSourceIndex(mergeCmd.getSourceIndex());
     fwdMerge->setPriority(mergeCmd.getPriority());