You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2018/11/06 22:38:31 UTC
[3/3] kudu git commit: [tools] reference comparison mode for
rebalancing algo tests
[tools] reference comparison mode for rebalancing algo tests
Introduced comparison mode for rebalancing algorithms' tests.
For current rebalancing algorithms, it's natural to re-order contiguous
moves of the same weight. That's because of:
* Iterating over elements of a hash container keyed by
the weight of a move.
* Randomly choosing among multiple options of the same weight.
This patch adds MovesOrderingComparison::IGNORE option into one test
configuration of the RebalanceAlgoUnitTest.LocationBalancingSimpleST
scenario. That fixes the breakage of the test on Ubuntu 18.
Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91
Reviewed-on: http://gerrit.cloudera.org:8080/11870
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/6ce61d6e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6ce61d6e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6ce61d6e
Branch: refs/heads/master
Commit: 6ce61d6e67f3cb54ed034fda9c5dd443970ea454
Parents: 8e9345a
Author: Alexey Serbin <al...@apache.org>
Authored: Fri Nov 2 12:06:29 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Tue Nov 6 22:28:12 2018 +0000
----------------------------------------------------------------------
src/kudu/tools/rebalance_algo-test.cc | 63 ++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/6ce61d6e/src/kudu/tools/rebalance_algo-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/rebalance_algo-test.cc b/src/kudu/tools/rebalance_algo-test.cc
index 3271339..6a5e5a8 100644
--- a/src/kudu/tools/rebalance_algo-test.cc
+++ b/src/kudu/tools/rebalance_algo-test.cc
@@ -17,6 +17,7 @@
#include "kudu/tools/rebalance_algo.h"
+#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <iostream>
@@ -67,6 +68,7 @@ using std::endl;
using std::ostream;
using std::ostringstream;
using std::set;
+using std::sort;
using std::string;
using std::unordered_map;
using std::vector;
@@ -84,6 +86,23 @@ struct TablePerServerReplicas {
const vector<size_t> num_replicas_by_server;
};
+// Whether the order of the moves in the reference results should be verified
+// against the actual moves.
+enum class MovesOrderingComparison {
+ IGNORE,
+ VERIFY,
+};
+struct ReferenceComparisonOptions {
+ // Constructor to initialize the options by default.
+ // NOLINTNEXTLINE(google-explicit-constructor)
+ ReferenceComparisonOptions(MovesOrderingComparison moves_ordering =
+ MovesOrderingComparison::VERIFY)
+ : moves_ordering(moves_ordering) {
+ }
+
+ const MovesOrderingComparison moves_ordering;
+};
+
// Structure to describe rebalancing-related state of the cluster expressively
// enough for the tests.
struct TestClusterConfig {
@@ -104,6 +123,9 @@ struct TestClusterConfig {
// The expected replica movements: the reference output of the algorithm
// to compare with.
const vector<TableReplicaMove> expected_moves;
+
+ // Options controlling how the reference and the actual results are compared.
+ const ReferenceComparisonOptions ref_comparison_options;
};
bool operator==(const TableReplicaMove& lhs, const TableReplicaMove& rhs) {
@@ -201,7 +223,40 @@ void VerifyLocationRebalancingMoves(const TestClusterConfig& cfg) {
LocationBalancingAlgo algo;
ASSERT_OK(algo.GetNextMoves(ci, 0, &moves));
}
- EXPECT_EQ(cfg.expected_moves, moves);
+ switch (cfg.ref_comparison_options.moves_ordering) {
+ case MovesOrderingComparison::IGNORE:
+ {
+ // The case when the order of moves is not important. For the
+ // rebalancing algorithms, it's natural to re-order contiguous moves
+ // of the same weight. This is because of:
+ // a) randomly choosing among multiple options of the same weight
+ // b) iterating over elements of a hash container keyed by the weight
+ // of a move.
+ // Here it's necessary to normalize both the reference and the actual
+ // results before performing element-to-element comparison.
+ vector<TableReplicaMove> ref_moves(cfg.expected_moves);
+ constexpr auto kMovesComparator = [](const TableReplicaMove& lhs,
+ const TableReplicaMove& rhs) {
+ if (lhs.table_id != rhs.table_id) {
+ return lhs.table_id < rhs.table_id;
+ }
+ if (lhs.from != rhs.from) {
+ return lhs.from < rhs.from;
+ }
+ return lhs.to < rhs.to;
+ };
+ sort(ref_moves.begin(), ref_moves.end(), kMovesComparator);
+ sort(moves.begin(), moves.end(), kMovesComparator);
+ EXPECT_EQ(ref_moves, moves);
+ }
+ break;
+ case MovesOrderingComparison::VERIFY:
+ EXPECT_EQ(cfg.expected_moves, moves);
+ break;
+ default:
+ FAIL() << "unexpected reference comparison style";
+ break;
+ }
}
// Is 'cbi' balanced according to the two-dimensional greedy algorithm?
@@ -984,13 +1039,13 @@ TEST(RebalanceAlgoUnitTest, LocationBalancingSimpleST) {
},
{ "0", "1", "2", },
{ { "A", { 6, 0, 0, } }, },
- // TODO(aserbin): what about ordering?
{
- { "A", "0", "2" },
{ "A", "0", "1" },
+ { "A", "0", "2" },
{ "A", "0", "1" },
{ "A", "0", "2" },
- }
+ },
+ { MovesOrderingComparison::IGNORE }
},
{
{