Minor C++ cleanups

6 files changed, 8 insertions, 7 deletions

diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp
index 7188d060b41..9795f5db5dc 100644
--- a/storage/src/tests/distributor/bucketdbupdatertest.cpp
+++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp
@@ -2674,7 +2674,6 @@ void for_each_bucket(const BucketDatabase& db, const document::BucketSpace& spac
 template <typename Func>
 void for_each_bucket(const DistributorBucketSpaceRepo& repo, Func&& f) {
     for (const auto& space : repo) {
-        // TODO needs to propagate space
         for_each_bucket(space.second->getBucketDatabase(), space.first, f);
     }
 }
diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp
index 76917f856f0..40fe885dcb1 100644
--- a/storage/src/tests/distributor/externaloperationhandlertest.cpp
+++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp
@@ -337,6 +337,8 @@ ExternalOperationHandlerTest::verify_busy_bounced_due_to_no_active_state(std::sh
             _sender.replies[0]->getResult().toString());
 }
 
+// TODO NOT_READY is a more appropriate return code for this case, but must ensure it's
+// handled gracefully and silently through the stack. BUSY is a safe bet until then.
 void
 ExternalOperationHandlerTest::mutating_operation_busy_bounced_if_no_cluster_state_received_yet()
 {
diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp
index 7516e10082c..4e99ea093af 100644
--- a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp
+++ b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp
@@ -121,8 +121,7 @@ void
 BucketDBUpdater::removeSuperfluousBuckets(
         const lib::ClusterStateBundle& newState)
 {
-    const bool move_to_read_only_db = _distributorComponent.getDistributor().getConfig()
-            .allowStaleReadsDuringClusterStateTransitions();
+    const bool move_to_read_only_db = shouldDeferStateEnabling();
     for (auto &elem : _distributorComponent.getBucketSpaceRepo()) {
         const auto &newDistribution(elem.second->getDistribution());
         const auto &oldClusterState(elem.second->getClusterState());
diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp
index 6184f8ff26a..c92dfbdc14e 100644
--- a/storage/src/vespa/storage/distributor/distributor.cpp
+++ b/storage/src/vespa/storage/distributor/distributor.cpp
@@ -576,10 +576,10 @@ void
 Distributor::propagateDefaultDistribution(
         std::shared_ptr<const lib::Distribution> distribution)
 {
+    auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(*distribution);
     for (auto* repo : {_bucketSpaceRepo.get(), _readOnlyBucketSpaceRepo.get()}) {
         repo->get(document::FixedBucketSpaces::default_space()).setDistribution(distribution);
-        auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(*distribution);
-        repo->get(document::FixedBucketSpaces::global_space()).setDistribution(std::move(global_distr));
+        repo->get(document::FixedBucketSpaces::global_space()).setDistribution(global_distr);
     }
 }
 
diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
index 7d0681df88d..1b88f02cac6 100644
--- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
+++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
@@ -79,11 +79,12 @@ void ExternalOperationHandler::bounce_with_result(api::StorageCommand& cmd, cons
 }
 
 void ExternalOperationHandler::bounce_with_wrong_distribution(api::StorageCommand& cmd) {
-    // Distributor ownership is equal across cluster states, so always send back default state.
+    // Distributor ownership is equal across bucket spaces, so always send back default space state.
     // This also helps client avoid getting confused by possibly observing different actual
     // (derived) state strings for global/non-global document types for the same state version.
     // Similarly, if we've yet to activate any version at all we send back BUSY instead
     // of a suspiciously empty WrongDistributionReply.
+    // TOOD consider NOT_READY instead of BUSY once we're sure this won't cause any other issues.
     const auto& cluster_state = _bucketSpaceRepo.get(document::FixedBucketSpaces::default_space()).getClusterState();
     if (cluster_state.getVersion() != 0) {
         auto cluster_state_str = cluster_state.toString();
diff --git a/storage/src/vespa/storage/storageserver/fnetlistener.cpp b/storage/src/vespa/storage/storageserver/fnetlistener.cpp
index fb2967965c0..ec488b25714 100644
--- a/storage/src/vespa/storage/storageserver/fnetlistener.cpp
+++ b/storage/src/vespa/storage/storageserver/fnetlistener.cpp
@@ -224,7 +224,7 @@ void FNetListener::RPC_activateClusterStateVersion(FRT_RPCRequest* req) {
         return;
     }
 
-    const int32_t activate_version = req->GetParams()->GetValue(0)._intval32;
+    const uint32_t activate_version = req->GetParams()->GetValue(0)._intval32;
     auto cmd = std::make_shared<api::ActivateClusterStateVersionCommand>(activate_version);
     cmd->setPriority(api::StorageMessage::VERYHIGH);