Remove no longer needed hash normalizer.

8 files changed, 5 insertions, 366 deletions

diff --git a/storage/src/tests/bucketdb/CMakeLists.txt b/storage/src/tests/bucketdb/CMakeLists.txt
index 5aa4e18cb84..13c9863aa8e 100644
--- a/storage/src/tests/bucketdb/CMakeLists.txt
+++ b/storage/src/tests/bucketdb/CMakeLists.txt
@@ -3,7 +3,6 @@ vespa_add_library(storage_testbucketdb TEST
     SOURCES
     bucketinfotest.cpp
     bucketmanagertest.cpp
-    distribution_hash_normalizer_test.cpp
     initializertest.cpp
     judyarraytest.cpp
     judymultimaptest.cpp
diff --git a/storage/src/tests/bucketdb/distribution_hash_normalizer_test.cpp b/storage/src/tests/bucketdb/distribution_hash_normalizer_test.cpp
deleted file mode 100644
index da7db8c6f4c..00000000000
--- a/storage/src/tests/bucketdb/distribution_hash_normalizer_test.cpp
+++ /dev/null
@@ -1,114 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include <vespa/vdstestlib/cppunit/macros.h>
-#include <vespa/storage/bucketdb/distribution_hash_normalizer.h>
-#include <string>
-
-namespace storage {
-
-using Normalizer = DistributionHashNormalizer;
-
-class DistributionHashNormalizerTest : public CppUnit::TestFixture {
-public:
-    CPPUNIT_TEST_SUITE(DistributionHashNormalizerTest);
-    CPPUNIT_TEST(orderNonHierarchicRootGroupNodesByDistributionKey);
-    CPPUNIT_TEST(mayHaveSameGroupIndexAsNodeIndex);
-    CPPUNIT_TEST(emitOptionalCapacityForRootGroup);
-    CPPUNIT_TEST(emitOptionalCapacityForSubGroups);
-    CPPUNIT_TEST(hierarchicGroupsAreOrderedByGroupIndex);
-    CPPUNIT_TEST(subgroupsOrderedOnEachNestingLevel);
-    CPPUNIT_TEST(distributionSpecIsCopiedVerbatim);
-    CPPUNIT_TEST(emptyInputYieldsEmptyOutput);
-    CPPUNIT_TEST(parseFailureReturnsInputVerbatim);
-    CPPUNIT_TEST_SUITE_END();
-
-    void orderNonHierarchicRootGroupNodesByDistributionKey();
-    void mayHaveSameGroupIndexAsNodeIndex();
-    void emitOptionalCapacityForRootGroup();
-    void emitOptionalCapacityForSubGroups();
-    void hierarchicGroupsAreOrderedByGroupIndex();
-    void subgroupsOrderedOnEachNestingLevel();
-    void distributionSpecIsCopiedVerbatim();
-    void emptyInputYieldsEmptyOutput();
-    void parseFailureReturnsInputVerbatim();
-
-private:
-    DistributionHashNormalizer _normalizer;
-};
-
-CPPUNIT_TEST_SUITE_REGISTRATION(DistributionHashNormalizerTest);
-
-void
-DistributionHashNormalizerTest::orderNonHierarchicRootGroupNodesByDistributionKey()
-{
-    // Group index is first in list.
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(1;0;2;3;4;7)"),
-                         _normalizer.normalize("(1;4;7;2;0;3)"));
-}
-
-void
-DistributionHashNormalizerTest::mayHaveSameGroupIndexAsNodeIndex()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0;0;2;3;4;7)"),
-                         _normalizer.normalize("(0;4;7;2;0;3)"));
-}
-
-void
-DistributionHashNormalizerTest::emitOptionalCapacityForRootGroup()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0c12.5;1;2;3;4;7)"),
-                         _normalizer.normalize("(0c12.5;1;4;7;2;3)"));
-}
-
-void
-DistributionHashNormalizerTest::emitOptionalCapacityForSubGroups()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0d1|*(1c5.5;1)(2;2)(3c7;3))"),
-                         _normalizer.normalize("(0d1|*(2;2)(1c5.5;1)(3c7;3))"));
-}
-
-void
-DistributionHashNormalizerTest::hierarchicGroupsAreOrderedByGroupIndex()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0d1|*(0;0)(1;1)(3;3))"),
-                         _normalizer.normalize("(0d1|*(3;3)(1;1)(0;0))"));
-}
-
-void
-DistributionHashNormalizerTest::subgroupsOrderedOnEachNestingLevel()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0d1|*(1d3|*(2;2)(3;3))"
-                                          "(4;1)(7d2|*(5;5)(6;6)))"),
-                         _normalizer.normalize("(0d1|*(7d2|*(6;6)(5;5))"
-                                               "(1d3|*(2;2)(3;3))(4;1))"));
-}
-
-void
-DistributionHashNormalizerTest::distributionSpecIsCopiedVerbatim()
-{
-    // Definitely don't want to do any ordering of the distribution spec.
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("(0d3|2|1|*(0;0)(1;1)(3;3))"),
-                         _normalizer.normalize("(0d3|2|1|*(3;3)(1;1)(0;0))"));
-}
-
-void
-DistributionHashNormalizerTest::emptyInputYieldsEmptyOutput()
-{
-    // Technically a parse failure (only 4.2 has this behavior), but it's
-    // explicitly checked for in BucketManager, so let's test it explicitly
-    // here as well.
-    CPPUNIT_ASSERT_EQUAL(vespalib::string(""), _normalizer.normalize(""));
-}
-
-// In the (unlikely) case that the parser somehow fails to capture all possible
-// valid values of the distribution hash, fall back to returning the non-
-// normalized string. A log warning will also be emitted (though that's not
-// testable).
-void
-DistributionHashNormalizerTest::parseFailureReturnsInputVerbatim()
-{
-    CPPUNIT_ASSERT_EQUAL(vespalib::string("onkel skrue"),
-                         _normalizer.normalize("onkel skrue"));
-}
-
-} // storage
-
diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp
index 28fccf438ff..a9b28bd2542 100644
--- a/storage/src/tests/distributor/distributortest.cpp
+++ b/storage/src/tests/distributor/distributortest.cpp
@@ -1,6 +1,5 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <boost/assign/std/vector.hpp> // for 'operator+=()'
 #include <vespa/vdstestlib/cppunit/macros.h>
 #include <vespa/storage/distributor/idealstatemetricsset.h>
 #include <vespa/storageapi/message/persistence.h>
diff --git a/storage/src/vespa/storage/bucketdb/CMakeLists.txt b/storage/src/vespa/storage/bucketdb/CMakeLists.txt
index 1b59ea2290b..8200884de17 100644
--- a/storage/src/vespa/storage/bucketdb/CMakeLists.txt
+++ b/storage/src/vespa/storage/bucketdb/CMakeLists.txt
@@ -6,7 +6,6 @@ vespa_add_library(storage_bucketdb OBJECT
     bucketinfo.cpp
     bucketmanager.cpp
     bucketmanagermetrics.cpp
-    distribution_hash_normalizer.cpp
     judyarray.cpp
     lockablemap.cpp
     mapbucketdatabase.cpp
diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
index 733cc490bd3..551f6fc726d 100644
--- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
+++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "bucketmanager.h"
-#include "distribution_hash_normalizer.h"
 #include "minimumusedbitstracker.h"
 #include "lockablemap.hpp"
 #include <iomanip>
@@ -522,10 +521,7 @@ BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpac
     lib::ClusterState::CSP clusterState(clusterStateBundle->getDerivedClusterState(bucketSpace));
     assert(clusterState.get());
 
-    DistributionHashNormalizer normalizer;
-
-    const auto our_hash = normalizer.normalize(
-            distribution->getNodeGraph().getDistributionConfigHash());
+    const auto our_hash = distribution->getNodeGraph().getDistributionConfigHash();
 
     LOG(debug, "Processing %" PRIu64 " queued request bucket info commands. "
         "Using cluster state '%s' and distribution hash '%s'",
@@ -537,8 +533,7 @@ BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpac
     for (auto it = reqs.rbegin(); it != reqs.rend(); ++it) {
         // Currently small requests should not be forwarded to worker thread
         assert((*it)->hasSystemState());
-        const auto their_hash = normalizer.normalize(
-                (*it)->getDistributionHash());
+        const auto their_hash = (*it)->getDistributionHash();
 
         std::ostringstream error;
         if ((*it)->getSystemState().getVersion() > _lastClusterStateSeen) {
@@ -570,8 +565,7 @@ BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpac
         
     	// If we get here, message should be failed
         auto reply = std::make_shared<api::RequestBucketInfoReply>(**it);
-        reply->setResult(api::ReturnCode(
-                    api::ReturnCode::REJECTED, error.str()));
+        reply->setResult(api::ReturnCode(api::ReturnCode::REJECTED, error.str()));
         LOG(debug, "Rejecting request from distributor %u: %s",
             (*it)->getDistributor(),
             error.str().c_str());
diff --git a/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.cpp b/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.cpp
deleted file mode 100644
index 1e6825555b2..00000000000
--- a/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.cpp
+++ /dev/null
@@ -1,206 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include "distribution_hash_normalizer.h"
-#include <vespa/vespalib/stllike/asciistream.h>
-#include <boost/spirit/include/qi.hpp>
-#include <boost/spirit/include/phoenix_core.hpp>
-#include <boost/spirit/include/phoenix_object.hpp>
-#include <boost/fusion/include/adapt_struct.hpp>
-#include <boost/optional.hpp>
-#include <boost/variant/recursive_wrapper.hpp>
-#include <vector>
-#include <algorithm>
-#include <iterator>
-#include <functional>
-
-#include <vespa/log/bufferedlogger.h>
-LOG_SETUP(".storage.bucketdb.distribution_hash_normalizer");
-
-// TODO
-// This code can be removed once we have a model out which ensures consistent
-// ordering of nodes in the stor-distribution config.
-
-namespace qi = boost::spirit::qi;
-namespace ascii = boost::spirit::ascii;
-namespace phoenix = boost::phoenix;
-
-namespace {
-
-struct GroupSet;
-
-using Children = boost::variant<
-    std::vector<unsigned int>,
-    boost::recursive_wrapper<GroupSet>
->;
-
-struct Group {
-    uint16_t index;
-    boost::optional<double> capacity;
-    Children children;
-    ~Group() {}
-};
-
-struct GroupSet {
-    std::string distribution_spec;
-    std::vector<Group> subgroups;
-};
-
-} // anon ns
-
-// Fusion adaptations must be in global scope.
-BOOST_FUSION_ADAPT_STRUCT(
-    ::Group,
-    (uint16_t, index)
-    (boost::optional<double>, capacity)
-    (Children, children)
-)
-
-BOOST_FUSION_ADAPT_STRUCT(
-    ::GroupSet,
-    (std::string, distribution_spec)
-    (std::vector<Group>, subgroups)
-)
-
-namespace storage {
-namespace {
-
-// Boost.Spirit v2 grammar for parsing the output of lib::Group::getConfigHash.
-template <typename Iterator>
-struct HashGrammar
-    : qi::grammar<Iterator, Group()>
-{
-    HashGrammar()
-        : HashGrammar::base_type(group)
-    {
-        using qi::uint_;
-        using qi::double_;
-        using ascii::char_;
-        /*
-         * This grammar makes the (reasonable) assumption that you can't have
-         * empty groups.
-         *
-         * Quick Spirit PEG DSL syntax primer for any two arbitrary parsers
-         * a and b (all subcomponents of parsers are themselves parsers):
-         *
-         *   'X'     : character literal match parser
-         *   a >> b  : a must be followed by b ("a b" in EBNF)
-         *   -a      : optional ("a?" in EBNF)
-         *   a | b   : a or b must match (same as in EBNF)
-         *   +a      : match 1 or more times ("a+" in EBNF)
-         *   *a      : kleene star; 0 or more times ("a*" in EBNF)
-         *   a - b   : difference; a but not b
-         *
-         * Please see Boost.Spirit docs on how these map to parser attributes
-         * (optional maps to boost::optional of nested attribute, + or kleene
-         * star maps to an iterable range (std::vector) of nested attributes,
-         * a | b maps to a boost::variant of the attributes of a and b,
-         * a >> b maps to a boost::tuple of the attributes and so on; usually
-         * fairly intuitive).
-         */
-        group =
-                 '('
-              >> uint_
-              >> -('c' >> double_)
-              >> ( +(';' >> uint_)
-                 | subgroups
-                 )
-              >> ')';
-
-        subgroups = ('d' >> distr_spec >> +group);
-
-        distr_spec = +(char_ - '('); // Match everything until open paren.
-    }
-
-    qi::rule<Iterator, Group()> group;
-    qi::rule<Iterator, GroupSet()> subgroups;
-    qi::rule<Iterator, std::string()> distr_spec;
-};
-
-template <typename Range, typename Predicate>
-auto ordered_by(const Range& range, Predicate pred) {
-    std::vector<typename Range::value_type> copy(
-            std::begin(range), std::end(range));
-    std::sort(copy.begin(), copy.end(), pred);
-    return copy;
-}
-
-void emit_normalized_groups(vespalib::asciistream& out, const Group& g);
-
-struct InOrderGroupVisitor : boost::static_visitor<void> {
-    vespalib::asciistream& _out;
-    InOrderGroupVisitor(vespalib::asciistream& out)
-        : _out(out)
-    {
-    }
-
-    void operator()(const std::vector<unsigned int>& nodes) const {
-        for (uint16_t node : ordered_by(nodes, std::less<void>())) {
-            _out << ';' << node;
-        }
-    }
-
-    void operator()(const GroupSet& gs) const {
-        _out << 'd' << gs.distribution_spec;
-        auto index_less_than = [](auto& lhs, auto& rhs) {
-            return lhs.index < rhs.index;
-        };
-        // Ordering will also copy nested subgroups, but the number of known
-        // Vespa installations with nested subgroups is currently somewhere
-        // around the high end of zero.
-        for (auto& g : ordered_by(gs.subgroups, index_less_than)) {
-            emit_normalized_groups(_out, g);
-        }
-    }
-};
-
-void emit_normalized_groups(vespalib::asciistream& out, const Group& g) {
-    out << '(' << g.index;
-    if (g.capacity) {
-        out << 'c' << *g.capacity;
-    }
-    boost::apply_visitor(InOrderGroupVisitor(out), g.children);
-    out << ')';
-}
-
-} // anon ns
-
-// We keep the grammar around across multiple normalized() calls because
-// constructing the grammar object(s) isn't free.
-struct DistributionHashNormalizer::ParserImpl {
-    using Iterator = vespalib::string::const_iterator;
-    HashGrammar<Iterator> grammar;
-};
-
-DistributionHashNormalizer::DistributionHashNormalizer()
-    : _impl(std::make_unique<ParserImpl>())
-{
-}
-
-// Required here because of incomplete ParserImpl in header.
-DistributionHashNormalizer::~DistributionHashNormalizer()
-{
-}
-
-vespalib::string
-DistributionHashNormalizer::normalize(vespalib::stringref hash) const
-{
-    Group root;
-
-    auto iter = hash.begin();
-    const bool ok = qi::parse(iter, hash.end(), _impl->grammar, root);
-    if (!ok || iter != hash.end()) {
-        vespalib::string hash_str = hash; // stringref might not be zero-term'd.
-        LOGBT(warning, hash_str.c_str(),
-              "Unable to parse compact distribution config "
-              "representation: '%s'",
-              hash_str.c_str());
-        return hash; // Fallback to input on parse failure.
-    }
-
-    vespalib::asciistream out;
-    emit_normalized_groups(out, root);
-
-    return out.str();
-}
-
-} // storage
-
diff --git a/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.h b/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.h
deleted file mode 100644
index a3a79542265..00000000000
--- a/storage/src/vespa/storage/bucketdb/distribution_hash_normalizer.h
+++ /dev/null
@@ -1,28 +0,0 @@
-// 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>
-
-namespace storage {
-
-/**
- * Utility class for "normalizing" a received distribution hash string into
- * a representation that is ordering invariant across group and node indices.
- *
- * All group indices and node indices will be returned in increasing order.
- *
- * In the case of a parser error the original string will be returned verbatim.
- */
-class DistributionHashNormalizer {
-    // PIMPL the parser to avoid Spirit deps in header file.
-    struct ParserImpl;
-    std::unique_ptr<ParserImpl> _impl;
-public:
-    DistributionHashNormalizer();
-    ~DistributionHashNormalizer();
-
-    vespalib::string normalize(vespalib::stringref hash) const;
-};
-
-} // storage
-
diff --git a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h
index 888b6d248bd..41878f09014 100644
--- a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h
+++ b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h
@@ -1,12 +1,11 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
+#include "prioritizedbucket.h"
 #include <vespa/storage/bucketdb/bucketdatabase.h>
-#include <vespa/storage/distributor/maintenance/prioritizedbucket.h>
 #include <boost/iterator/iterator_facade.hpp>
 
-namespace storage {
-namespace distributor {
+namespace storage::distributor {
 
 class BucketPriorityDatabase
 {
@@ -69,7 +68,4 @@ public:
 };
 
 }
-}
-
-