Merge pull request #4328 from vespa-engine/vekterli/add-global-distribution-transformation-utility

Add global distribution config transformation utility

7 files changed, 78 insertions, 68 deletions

diff --git a/vdslib/src/vespa/vdslib/distribution/CMakeLists.txt b/vdslib/src/vespa/vdslib/distribution/CMakeLists.txt
index 86406937dd2..44693b42fe8 100644
--- a/vdslib/src/vespa/vdslib/distribution/CMakeLists.txt
+++ b/vdslib/src/vespa/vdslib/distribution/CMakeLists.txt
@@ -2,6 +2,7 @@
 vespa_add_library(vdslib_distribution OBJECT
     SOURCES
     distribution.cpp
+    distribution_config_util.cpp
     group.cpp
     idealnodecalculatorimpl.cpp
     redundancygroupdistribution.cpp
diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.cpp b/vdslib/src/vespa/vdslib/distribution/distribution.cpp
index 908de49e311..ff84687210d 100644
--- a/vdslib/src/vespa/vdslib/distribution/distribution.cpp
+++ b/vdslib/src/vespa/vdslib/distribution/distribution.cpp
@@ -1,18 +1,19 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "distribution.h"
+#include <vespa/vdslib/distribution/distribution_config_util.h>
 #include <vespa/vdslib/state/clusterstate.h>
 #include <vespa/vdslib/state/random.h>
-#include <vespa/vespalib/text/stringtokenizer.h>
 #include <vespa/vespalib/util/bobhash.h>
 #include <vespa/vespalib/stllike/asciistream.h>
-#include <boost/lexical_cast.hpp>
 #include <vespa/config/config.h>
 #include <vespa/config/print/asciiconfigwriter.h>
 #include <vespa/config/print/asciiconfigreader.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/config-stor-distribution.h>
 #include <list>
+#include <algorithm>
+#include <cmath>
 
 #include <vespa/log/bufferedlogger.h>
 LOG_SETUP(".vdslib.distribution");
@@ -115,17 +116,6 @@ Distribution::operator=(const Distribution& d)
 Distribution::~Distribution() { }
 
 namespace {
-    std::vector<uint16_t> getGroupPath(const vespalib::stringref & path) {
-        vespalib::StringTokenizer st(path, ".", "");
-        std::vector<uint16_t> result(st.size());
-        for (uint32_t i=0, n=result.size(); i<n; ++i) {
-            result[i] = boost::lexical_cast<uint16_t>(st[i]);
-        }
-        return result;
-    }
-}
-
-namespace {
     using ConfigDiskDistribution = vespa::config::content::StorDistributionConfig::DiskDistribution;
     Distribution::DiskDistribution fromConfig(ConfigDiskDistribution cfg) {
         switch (cfg) {
@@ -155,8 +145,8 @@ Distribution::configure(const vespa::config::content::StorDistributionConfig& co
     for (uint32_t i=0, n=config.group.size(); i<n; ++i) {
         const ConfigGroup& cg(config.group[i]);
         std::vector<uint16_t> path;
-        if (nodeGraph.get() != 0) {
-            path = getGroupPath(cg.index);
+        if (nodeGraph.get() != nullptr) {
+            path = DistributionConfigUtil::getGroupPath(cg.index);
         }
         bool isLeafGroup = (cg.nodes.size() > 0);
         std::unique_ptr<Group> group;
@@ -179,7 +169,7 @@ Distribution::configure(const vespa::config::content::StorDistributionConfig& co
         if (path.empty()) {
             nodeGraph = std::move(group);
         } else {
-            assert(nodeGraph.get() != 0);
+            assert(nodeGraph.get() != nullptr);
             Group* parent = nodeGraph.get();
             for (uint32_t j=0; j<path.size() - 1; ++j) {
                 parent = parent->getSubGroups()[path[j]];
@@ -187,7 +177,7 @@ Distribution::configure(const vespa::config::content::StorDistributionConfig& co
             parent->addSubGroup(std::move(group));
         }
     }
-    if (nodeGraph.get() == 0) {
+    if (nodeGraph.get() == nullptr) {
         throw vespalib::IllegalStateException(
             "Got config that didn't seem to specify even a root group. Must "
             "have a root group at minimum:\n"
@@ -462,14 +452,14 @@ Distribution::getIdealGroups(const document::BucketId& bucket,
             tmpResults.pop_back();
         }
     }
-    while (tmpResults.back()._group == 0) {
+    while (tmpResults.back()._group == nullptr) {
         tmpResults.pop_back();
     }
     for (uint32_t i=0, n=tmpResults.size(); i<n; ++i) {
         ScoredGroup& group(tmpResults[i]);
             // This should never happen. Config should verify that each group
             // has enough groups beneath them.
-        assert(group._group != 0);
+        assert(group._group != nullptr);
         getIdealGroups(bucket, clusterState, *group._group,
                        redundancyArray[i], results);
     }
@@ -478,14 +468,11 @@ Distribution::getIdealGroups(const document::BucketId& bucket,
 const Group*
 Distribution::getIdealDistributorGroup(const document::BucketId& bucket,
                                        const ClusterState& clusterState,
-                                       const Group& parent,
-                                       uint16_t redundancy) const
+                                       const Group& parent) const
 {
     if (parent.isLeafGroup()) {
         return &parent;
     }
-    const Group::Distribution& redundancyArray(
-            parent.getDistribution(redundancy));
     ScoredGroup result(0, 0);
     uint32_t seed(getGroupSeed(bucket, clusterState, parent));
     RandomGen random(seed);
@@ -497,8 +484,8 @@ Distribution::getIdealDistributorGroup(const document::BucketId& bucket,
         while (it->first < currentIndex++) random.nextDouble();
         double score = random.nextDouble();
         if (it->second->getCapacity() != 1) {
-                // Capacity shouldn't possibly be 0.
-                // Verified in Group::setCapacity()
+            // Capacity shouldn't possibly be 0.
+            // Verified in Group::setCapacity()
             score = std::pow(score, 1.0 / it->second->getCapacity().getValue());
         }
         if (score > result._score) {
@@ -509,11 +496,10 @@ Distribution::getIdealDistributorGroup(const document::BucketId& bucket,
             }
         }
     }
-    if (result._group == 0) {
-        return 0;
+    if (result._group == nullptr) {
+        return nullptr;
     }
-    return getIdealDistributorGroup(
-            bucket, clusterState, *result._group, redundancyArray[0]);
+    return getIdealDistributorGroup(bucket, clusterState, *result._group);
 }
 
 bool
@@ -567,9 +553,8 @@ Distribution::getIdealNodes(const NodeType& nodeType,
                        _groupDistribution);
     } else {
         seed = getDistributorSeed(bucket, clusterState);
-        const Group* group(getIdealDistributorGroup(
-                    bucket, clusterState, *_nodeGraph, redundancy));
-        if (group == 0) {
+        const Group* group(getIdealDistributorGroup(bucket, clusterState, *_nodeGraph));
+        if (group == nullptr) {
             vespalib::asciistream ss;
             ss << "There is no legal distributor target in state with version "
                << clusterState.getVersion();
@@ -679,7 +664,7 @@ Distribution::getIdealDistributorNode(
     std::vector<uint16_t> nodes;
     getIdealNodes(NodeType::DISTRIBUTOR, state, bucket, nodes, upStates);
     assert(nodes.size() <= 1);
-    if (nodes.size() == 0) {
+    if (nodes.empty()) {
         vespalib::asciistream ss;
         ss << "There is no legal distributor target in state with version "
            << state.getVersion();
@@ -695,7 +680,7 @@ Distribution::splitNodesIntoLeafGroups(IndexList nodeList) const
     std::map<uint16_t, IndexList> nodes;
     for (uint32_t i=0, n=nodeList.size(); i<n; ++i) {
         const Group* group(_nodeGraph->getGroupForNode(nodeList[i]));
-        if (group == 0) {
+        if (group == nullptr) {
             LOGBP(warning, "Node %u is not assigned to a group. "
                            "Should not happen?", nodeList[i]);
         } else {
diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.h b/vdslib/src/vespa/vdslib/distribution/distribution.h
index c828dbff9b8..6da60e084bb 100644
--- a/vdslib/src/vespa/vdslib/distribution/distribution.h
+++ b/vdslib/src/vespa/vdslib/distribution/distribution.h
@@ -13,17 +13,10 @@
 #include <vespa/vdslib/state/nodetype.h>
 #include <vespa/vespalib/util/exception.h>
 
-namespace vespa {
-    namespace config {
-        namespace content {
-            namespace internal {
-                class InternalStorDistributionType;
-            }
-        }
-    }
+namespace vespa::config::content::internal {
+    class InternalStorDistributionType;
 }
-namespace storage {
-namespace lib {
+namespace storage::lib {
 
 VESPA_DEFINE_EXCEPTION(NoDistributorsAvailableException, vespalib::Exception);
 VESPA_DEFINE_EXCEPTION(TooFewBucketBitsInUseException, vespalib::Exception);
@@ -101,8 +94,7 @@ private:
 
     const Group* getIdealDistributorGroup(const document::BucketId& bucket,
                                           const ClusterState& clusterState,
-                                          const Group& parent,
-                                          uint16_t redundancy) const;
+                                          const Group& parent) const;
 
     /**
      * Since distribution object may be used often in ideal state calculations
@@ -205,6 +197,5 @@ public:
     static bool allDistributorsDown(const Group&, const ClusterState&);
 };
 
-} // lib
-} // storage
+} // storage::lib
 
diff --git a/vdslib/src/vespa/vdslib/distribution/distribution_config_util.cpp b/vdslib/src/vespa/vdslib/distribution/distribution_config_util.cpp
new file mode 100644
index 00000000000..e700dd4e379
--- /dev/null
+++ b/vdslib/src/vespa/vdslib/distribution/distribution_config_util.cpp
@@ -0,0 +1,18 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "distribution_config_util.h"
+#include <vespa/vespalib/text/stringtokenizer.h>
+#include <boost/lexical_cast.hpp>
+
+namespace storage::lib {
+
+std::vector<uint16_t> DistributionConfigUtil::getGroupPath(vespalib::stringref path) {
+    vespalib::StringTokenizer st(path, ".", "");
+    std::vector<uint16_t> result(st.size());
+    for (uint32_t i=0, n=result.size(); i<n; ++i) {
+        result[i] = boost::lexical_cast<uint16_t>(st[i]);
+    }
+    return result;
+}
+
+}
diff --git a/vdslib/src/vespa/vdslib/distribution/distribution_config_util.h b/vdslib/src/vespa/vdslib/distribution/distribution_config_util.h
new file mode 100644
index 00000000000..3466b16f275
--- /dev/null
+++ b/vdslib/src/vespa/vdslib/distribution/distribution_config_util.h
@@ -0,0 +1,15 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include <vespa/vespalib/stllike/string.h>
+#include <vector>
+
+namespace storage::lib {
+
+struct DistributionConfigUtil {
+    // Converts an input string of the form "1.2.3" to a returned vector {1, 2, 3}
+    static std::vector<uint16_t> getGroupPath(vespalib::stringref path);
+};
+
+}
diff --git a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp
index 97bde0ca54f..f620dc15928 100644
--- a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp
+++ b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp
@@ -14,10 +14,10 @@ namespace {
     void verifyLegal(vespalib::StringTokenizer& st,
                      vespalib::stringref serialized)
     {
-            // First, verify sanity of the serialized string
-        uint32_t firstAsterix = st.size();
+        // First, verify sanity of the serialized string
+        uint32_t firstAsterisk = st.size();
         for (uint32_t i=0; i<st.size(); ++i) {
-            if (i > firstAsterix) {
+            if (i > firstAsterisk) {
                 if (st[i] != "*") {
                     throw vespalib::IllegalArgumentException(
                             "Illegal distribution spec \"" + serialized + "\". "
@@ -26,8 +26,8 @@ namespace {
                 }
                 continue;
             }
-            if (i < firstAsterix && st[i] == "*") {
-                firstAsterix = i;
+            if (i < firstAsterisk && st[i] == "*") {
+                firstAsterisk = i;
                 continue;
             }
             uint32_t number = atoi(st[i].c_str());
@@ -76,21 +76,21 @@ RedundancyGroupDistribution::RedundancyGroupDistribution(
         const RedundancyGroupDistribution& spec,
         uint16_t redundancy)
 {
-    uint16_t firstAsterix = spec.getFirstAsterixIndex();
-        // If redundancy is less than the group size, we only get one copy
-        // in redundancy groups.
+    uint16_t firstAsterisk = spec.getFirstAsteriskIndex();
+    // If redundancy is less than the group size, we only get one copy
+    // in redundancy groups.
     if (redundancy <= spec.size()) {
         _values = std::vector<uint16_t>(redundancy, 1);
         return;
     }
-        // If not we will have one copy at least for every wanted group.
+    // If not we will have one copy at least for every wanted group.
     _values = std::vector<uint16_t>(spec.size(), 1);
     redundancy -= spec.size();
-        // Distribute extra copies to non-asterix entries first
-    redundancy = divideSpecifiedCopies(0, firstAsterix, redundancy, spec._values);
-        // Distribute remaining copies to asterix entries
-    divideSpecifiedCopies(firstAsterix, spec.size(), redundancy, spec._values);
-        // Lastly sort, so the most copies will end up first in ideal state
+    // Distribute extra copies to non-asterisk entries first
+    redundancy = divideSpecifiedCopies(0, firstAsterisk, redundancy, spec._values);
+    // Distribute remaining copies to asterisk entries
+    divideSpecifiedCopies(firstAsterisk, spec.size(), redundancy, spec._values);
+    // Lastly sort, so the most copies will end up first in ideal state
     std::sort(_values.begin(), _values.end());
     std::reverse(_values.begin(), _values.end());
     assert(_values.front() >= _values.back());
@@ -111,18 +111,18 @@ RedundancyGroupDistribution::print(std::ostream& out,
 }
 
 uint16_t
-RedundancyGroupDistribution::getFirstAsterixIndex() const
+RedundancyGroupDistribution::getFirstAsteriskIndex() const
 {
     if (_values.empty() || _values.back() != 0) {
         throw vespalib::IllegalArgumentException(
                 "Invalid spec given. No asterisk entries found.",
                 VESPA_STRLOC);
     }
-    uint16_t firstAsterix = _values.size() - 1;
-    while (firstAsterix > 0 && _values[firstAsterix - 1] == 0) {
-        --firstAsterix;
+    uint16_t firstAsterisk = _values.size() - 1;
+    while (firstAsterisk > 0 && _values[firstAsterisk - 1] == 0) {
+        --firstAsterisk;
     }
-    return firstAsterix;
+    return firstAsterisk;
 }
 
 uint16_t
diff --git a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.h b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.h
index 5dde7cf378d..33f895cadf0 100644
--- a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.h
+++ b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.h
@@ -41,7 +41,7 @@ public:
     void print(std::ostream&, bool verbose, const std::string& indent) const override;
 
 private:
-    uint16_t getFirstAsterixIndex() const;
+    uint16_t getFirstAsteriskIndex() const;
     uint16_t divideSpecifiedCopies(
             uint16_t start, uint16_t end,
             uint16_t redundancy, const std::vector<uint16_t>& maxValues);