diff options
author | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2021-03-02 13:14:32 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2021-03-02 13:14:32 +0000 |
commit | de374ebb0cff7c612bdb41c9b7c1b3f31d46f03f (patch) | |
tree | 6a0820b5b395ad6d9dcec32dea4a0bd941414ec7 /vdslib | |
parent | 616be2724603dfac17d45db8eccd52369988a7d9 (diff) |
Remove notion of node-specific reliability from C++ distribution code
I have never seen this in use anywhere, and can find no code that ever
sets it. Bonus is that node candidate trimming can be vastly simplified.
Diffstat (limited to 'vdslib')
-rw-r--r-- | vdslib/src/tests/state/clusterstatetest.cpp | 22 | ||||
-rw-r--r-- | vdslib/src/tests/state/nodestatetest.cpp | 16 | ||||
-rw-r--r-- | vdslib/src/vespa/vdslib/distribution/distribution.cpp | 49 | ||||
-rw-r--r-- | vdslib/src/vespa/vdslib/state/nodestate.cpp | 59 | ||||
-rw-r--r-- | vdslib/src/vespa/vdslib/state/nodestate.h | 5 |
5 files changed, 32 insertions, 119 deletions
diff --git a/vdslib/src/tests/state/clusterstatetest.cpp b/vdslib/src/tests/state/clusterstatetest.cpp index 1880683232d..2dda26f8882 100644 --- a/vdslib/src/tests/state/clusterstatetest.cpp +++ b/vdslib/src/tests/state/clusterstatetest.cpp @@ -87,8 +87,8 @@ TEST(ClusterStateTest, test_basic_functionality) // Test other storage node propertise // (Messages is excluded from system states to not make them too long as // most nodes have no use for them) - VERIFYNEW("storage:9 .3.c:2.3 .4.r:8 .7.m:foo\\x20bar", - "storage:9 .3.c:2.3 .4.r:8"); + VERIFYNEW("storage:9 .3.c:2.3 .7.m:foo\\x20bar", + "storage:9 .3.c:2.3"); // Test that messages are kept in verbose mode, even if last index { @@ -159,7 +159,7 @@ TEST(ClusterStateTest, test_detailed) ClusterState state( "version:314 cluster:i " "distributor:8 .1.s:i .3.s:i .3.i:0.5 .5.s:d .7.m:foo\\x20bar " - "storage:10 .2.d:16 .2.d.3:d .4.s:d .5.c:1.3 .5.r:4" + "storage:10 .2.d:16 .2.d.3:d .4.s:d .5.c:1.3 " " .6.m:bar\\tfoo .7.s:m .8.d:10 .8.d.4.c:0.6 .8.d.4.m:small" ); EXPECT_EQ(314u, state.getVersion()); @@ -170,7 +170,7 @@ TEST(ClusterStateTest, test_detailed) // Testing distributor node states for (uint16_t i = 0; i <= 20; ++i) { const NodeState& ns(state.getNodeState(Node(NodeType::DISTRIBUTOR, i))); - // Test node states + // Test node states if (i == 1 || i == 3) { EXPECT_EQ(State::INITIALIZING, ns.getState()); } else if (i == 5 || i >= 8) { @@ -178,7 +178,7 @@ TEST(ClusterStateTest, test_detailed) } else { EXPECT_EQ(State::UP, ns.getState()); } - // Test initialize progress + // Test initialize progress if (i == 1) { EXPECT_EQ(vespalib::Double(0.0), ns.getInitProgress()); } else if (i == 3) { @@ -186,7 +186,7 @@ TEST(ClusterStateTest, test_detailed) } else { EXPECT_EQ(vespalib::Double(0.0), ns.getInitProgress()); } - // Test message + // Test message if (i == 7) { EXPECT_EQ(string("foo bar"), ns.getDescription()); } else { @@ -197,7 +197,7 @@ TEST(ClusterStateTest, test_detailed) // Testing storage node states for (uint16_t i = 0; i <= 20; ++i) { const NodeState& ns(state.getNodeState(Node(NodeType::STORAGE, i))); - // Test node states + // Test node states if (i == 4 || i >= 10) { EXPECT_EQ(State::DOWN, ns.getState()); } else if (i == 7) { @@ -211,13 +211,7 @@ TEST(ClusterStateTest, test_detailed) } else { EXPECT_EQ(string(""), ns.getDescription()); } - // Test reliability - if (i == 5) { - EXPECT_EQ(uint16_t(4), ns.getReliability()); - } else { - EXPECT_EQ(uint16_t(1), ns.getReliability()); - } - // Test capacity + // Test capacity if (i == 5) { EXPECT_EQ(vespalib::Double(1.3), ns.getCapacity()); } else { diff --git a/vdslib/src/tests/state/nodestatetest.cpp b/vdslib/src/tests/state/nodestatetest.cpp index da6faa49779..370c20eef8d 100644 --- a/vdslib/src/tests/state/nodestatetest.cpp +++ b/vdslib/src/tests/state/nodestatetest.cpp @@ -11,13 +11,11 @@ TEST(NodeStateTest, test_parsing) NodeState ns = NodeState("s:u"); EXPECT_EQ(std::string("s:u"), ns.toString()); EXPECT_EQ(vespalib::Double(1.0), ns.getCapacity()); - EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("s:m"); EXPECT_EQ(std::string("s:m"), ns.toString()); EXPECT_EQ(vespalib::Double(1.0), ns.getCapacity()); - EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("t:4"); @@ -25,31 +23,27 @@ TEST(NodeStateTest, test_parsing) EXPECT_EQ(uint64_t(4), ns.getStartTimestamp()); } { - NodeState ns = NodeState("s:u c:2.4 r:3 b:12"); - EXPECT_EQ(std::string("s:u c:2.4 r:3 b:12"), ns.toString()); + NodeState ns = NodeState("s:u c:2.4 b:12"); + EXPECT_EQ(std::string("s:u c:2.4 b:12"), ns.toString()); EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); - EXPECT_EQ(uint16_t(3), ns.getReliability()); EXPECT_EQ(12, (int)ns.getMinUsedBits()); EXPECT_NE(NodeState("s:u b:12"), NodeState("s:u b:13")); } { - NodeState ns = NodeState("c:2.4\ns:u\nr:5"); - EXPECT_EQ(std::string("s:u c:2.4 r:5"), ns.toString()); + NodeState ns = NodeState("c:2.4\ns:u"); + EXPECT_EQ(std::string("s:u c:2.4"), ns.toString()); EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); - EXPECT_EQ(uint16_t(5), ns.getReliability()); } { - NodeState ns = NodeState("c:2.4 r:1"); + NodeState ns = NodeState("c:2.4"); EXPECT_EQ(std::string("s:u c:2.4"), ns.toString()); EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); - EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("c:2.4 k:2.6"); EXPECT_EQ(std::string("s:u c:2.4"), ns.toString()); EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); - EXPECT_EQ(uint16_t(1), ns.getReliability()); } } diff --git a/vdslib/src/vespa/vdslib/distribution/distribution.cpp b/vdslib/src/vespa/vdslib/distribution/distribution.cpp index d51e9ee4f49..c5b9ec855d9 100644 --- a/vdslib/src/vespa/vdslib/distribution/distribution.cpp +++ b/vdslib/src/vespa/vdslib/distribution/distribution.cpp @@ -225,50 +225,27 @@ namespace { /** Used to record scored nodes during ideal nodes calculation. */ struct ScoredNode { - double _score; + double _score; uint16_t _index; - uint16_t _reliability; - ScoredNode() noexcept : _score(0), _index(0), _reliability(0) {} - ScoredNode(double score, uint16_t index, uint16_t reliability) noexcept - : _score(score), _index(index), _reliability(reliability) {} + constexpr ScoredNode() noexcept : _score(0), _index(UINT16_MAX) {} + constexpr ScoredNode(double score, uint16_t index) noexcept + : _score(score), _index(index) {} - bool operator<(const ScoredNode& other) const noexcept { + constexpr bool operator<(const ScoredNode& other) const noexcept { return (_score < other._score); } + constexpr bool valid() const noexcept { + return (_index != UINT16_MAX); + } }; - /** - * Throw away last entries until throwing away another would - * decrease redundancy below total reliability. If redundancy != - * total reliability, see if non-last entries can be removed. - */ + // Trim the input vector so that no trailing invalid entries remain and that + // it has a maximum size of `redundancy`. void trimResult(std::vector<ScoredNode>& nodes, uint16_t redundancy) { - // Initially record total reliability and use the first elements - // until satisfied. - uint32_t totalReliability = 0; - for (auto it = nodes.begin(); it != nodes.end(); ++it) { - if (totalReliability >= redundancy || it->_reliability == 0) { - nodes.erase(it, nodes.end()); - break; - } - totalReliability += it->_reliability; - } - // If we have too high reliability, see if we can remove something - // else - if (totalReliability > redundancy) { - for (auto it = nodes.rbegin(); it != nodes.rend();) { - if (it->_reliability <= (totalReliability - redundancy)) { - totalReliability -= it->_reliability; - auto deleteIt(it.base()); - ++it; - nodes.erase(--deleteIt); - if (totalReliability == redundancy) break; - } else { - ++it; - } - } + while (!nodes.empty() && (!nodes.back().valid() || (nodes.size() > redundancy))) { + nodes.pop_back(); } } @@ -456,7 +433,7 @@ Distribution::getIdealNodes(const NodeType& nodeType, score = std::pow(score, 1.0 / nodeState.getCapacity().getValue()); } if (score > tmpResults.back()._score) { - insertOrdered(tmpResults, ScoredNode(score, nodes[j], nodeState.getReliability())); + insertOrdered(tmpResults, ScoredNode(score, nodes[j])); } } trimResult(tmpResults, groupRedundancy); diff --git a/vdslib/src/vespa/vdslib/state/nodestate.cpp b/vdslib/src/vespa/vdslib/state/nodestate.cpp index fc4e18fa0cc..96ba4ac556f 100644 --- a/vdslib/src/vespa/vdslib/state/nodestate.cpp +++ b/vdslib/src/vespa/vdslib/state/nodestate.cpp @@ -26,7 +26,6 @@ NodeState::NodeState() _state(0), _description(""), _capacity(1.0), - _reliability(1), _initProgress(0.0), _minUsedBits(16), _startTimestamp(0) @@ -35,13 +34,11 @@ NodeState::NodeState() } NodeState::NodeState(const NodeType& type, const State& state, - vespalib::stringref description, - double capacity, uint16_t reliability) + vespalib::stringref description, double capacity) : _type(&type), _state(0), _description(description), _capacity(1.0), - _reliability(1), _initProgress(0.0), _minUsedBits(16), _startTimestamp(0) @@ -49,7 +46,6 @@ NodeState::NodeState(const NodeType& type, const State& state, setState(state); if (type == NodeType::STORAGE) { setCapacity(capacity); - setReliability(reliability); } } @@ -58,7 +54,6 @@ NodeState::NodeState(vespalib::stringref serialized, const NodeType* type) _state(&State::UP), _description(), _capacity(1.0), - _reliability(1), _initProgress(0.0), _minUsedBits(16), _startTimestamp(0) @@ -105,18 +100,6 @@ NodeState::NodeState(vespalib::stringref serialized, const NodeType* type) "a positive floating point number", VESPA_STRLOC); } continue; - case 'r': - if (_type != 0 && *type != NodeType::STORAGE) break; - if (key.size() > 1) break; - try{ - setReliability(boost::lexical_cast<uint16_t>(value)); - } catch (...) { - throw vespalib::IllegalArgumentException( - "Illegal reliability '" + value + "'. Reliability " - "must be a positive integer", - VESPA_STRLOC); - } - continue; case 'i': if (key.size() > 1) break; try{ @@ -180,8 +163,8 @@ NodeState::serialize(vespalib::asciistream & out, vespalib::stringref prefix, bool includeDescription) const { SeparatorPrinter sep; - // Always give node state if not part of a system state - // to prevent empty serialization + // Always give node state if not part of a system state + // to prevent empty serialization if (*_state != State::UP || prefix.size() == 0) { out << sep << prefix << "s:"; out << _state->serialize(); @@ -189,9 +172,6 @@ NodeState::serialize(vespalib::asciistream & out, vespalib::stringref prefix, if (_capacity != 1.0) { out << sep << prefix << "c:" << _capacity; } - if (_reliability != 1) { - out << sep << prefix << "r:" << _reliability; - } if (_minUsedBits != 16) { out << sep << prefix << "b:" << _minUsedBits; } @@ -211,8 +191,8 @@ void NodeState::setState(const State& state) { if (_type != 0) { - // We don't know whether you want to store reported, wanted or - // current node state, so we must accept any. + // We don't know whether you want to store reported, wanted or + // current node state, so we must accept any. if (!state.validReportedNodeState(*_type) && !state.validWantedNodeState(*_type)) { @@ -253,22 +233,6 @@ NodeState::setCapacity(vespalib::Double capacity) } void -NodeState::setReliability(uint16_t reliability) -{ - if (reliability == 0) { - std::ostringstream ost; - ost << "Illegal reliability '" << reliability << "'. Reliability " - "must be a positive integer."; - throw vespalib::IllegalArgumentException(ost.str(), VESPA_STRLOC); - } - if (_type != 0 && *_type != NodeType::STORAGE) { - throw vespalib::IllegalArgumentException( - "Reliability only make sense for storage nodes.", VESPA_STRLOC); - } - _reliability = reliability; -} - -void NodeState::setInitProgress(vespalib::Double initProgress) { if (initProgress < 0 || initProgress > 1.0) { @@ -300,9 +264,6 @@ NodeState::print(std::ostream& out, bool verbose, if (_capacity != 1.0) { out << ", capacity " << _capacity; } - if (_reliability != 1) { - out << ", reliability " << _reliability; - } if (_minUsedBits != 16) { out << ", minimum used bits " << _minUsedBits; } @@ -322,7 +283,6 @@ NodeState::operator==(const NodeState& other) const { if (_state != other._state || _capacity != other._capacity || - _reliability != other._reliability || _minUsedBits != other._minUsedBits || _startTimestamp != other._startTimestamp || (*_state == State::INITIALIZING @@ -338,7 +298,6 @@ NodeState::similarTo(const NodeState& other) const { if (_state != other._state || _capacity != other._capacity || - _reliability != other._reliability || _minUsedBits != other._minUsedBits || _startTimestamp < other._startTimestamp) { @@ -370,10 +329,6 @@ NodeState::verifySupportForNodeType(const NodeType& type) const throw vespalib::IllegalArgumentException("Capacity should not be " "set for a distributor node.", VESPA_STRLOC); } - if (type == NodeType::DISTRIBUTOR && _reliability != 1) { - throw vespalib::IllegalArgumentException("Reliability should not be " - "set for a distributor node.", VESPA_STRLOC); - } } std::string @@ -389,10 +344,6 @@ NodeState::getTextualDifference(const NodeState& other) const { source << ", capacity " << _capacity; target << ", capacity " << other._capacity; } - if (_reliability != other._reliability) { - source << ", reliability " << _reliability; - target << ", reliability " << other._reliability; - } if (_minUsedBits != other._minUsedBits) { source << ", minUsedBits " << _minUsedBits; target << ", minUsedBits " << _minUsedBits; diff --git a/vdslib/src/vespa/vdslib/state/nodestate.h b/vdslib/src/vespa/vdslib/state/nodestate.h index 685030e948f..f0ae7012dcb 100644 --- a/vdslib/src/vespa/vdslib/state/nodestate.h +++ b/vdslib/src/vespa/vdslib/state/nodestate.h @@ -23,7 +23,6 @@ class NodeState : public document::Printable const State* _state; vespalib::string _description; vespalib::Double _capacity; - uint16_t _reliability; vespalib::Double _initProgress; uint32_t _minUsedBits; uint64_t _startTimestamp; @@ -42,7 +41,7 @@ public: NodeState & operator = (NodeState &&) noexcept; NodeState(const NodeType& nodeType, const State&, vespalib::stringref description = "", - double capacity = 1.0, uint16_t reliability = 1); + double capacity = 1.0); /** Set type if you want to verify that content fit with the given type. */ NodeState(vespalib::stringref serialized, const NodeType* nodeType = 0); ~NodeState(); @@ -58,7 +57,6 @@ public: const State& getState() const { return *_state; } vespalib::Double getCapacity() const { return _capacity; } uint32_t getMinUsedBits() const { return _minUsedBits; } - uint16_t getReliability() const { return _reliability; } vespalib::Double getInitProgress() const { return _initProgress; } const vespalib::string& getDescription() const { return _description; } uint64_t getStartTimestamp() const { return _startTimestamp; } @@ -66,7 +64,6 @@ public: void setState(const State& state); void setCapacity(vespalib::Double capacity); void setMinUsedBits(uint32_t usedBits); - void setReliability(uint16_t reliability); void setInitProgress(vespalib::Double initProgress); void setStartTimestamp(uint64_t startTimestamp); void setDescription(vespalib::stringref desc) { _description = desc; } |