kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [3/3] kudu git commit: [tools] reference comparison mode for rebalancing algo tests
Date Tue, 06 Nov 2018 22:38:31 GMT
[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 <wdberkeley@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 <alexey@apache.org>
Authored: Fri Nov 2 12:06:29 2018 -0700
Committer: Alexey Serbin <aserbin@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 }
     },
     {
       {


Mime
View raw message