Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11549 )
Change subject: [rebalancer] location-aware rebalancer (part 1/n) ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/11549/7/src/kudu/tools/placement_policy_util-test.cc File src/kudu/tools/placement_policy_util-test.cc: http://gerrit.cloudera.org:8080/#/c/11549/7/src/kudu/tools/placement_policy_util-test.cc@374 PS7, Line 374: // Two tables, each of two RF=3 tablets with replica distribution violating : // the placement policy. > This comment doesn't match the test case. The test case has two tables, one That's a good catch! http://gerrit.cloudera.org:8080/#/c/11549/7/src/kudu/tools/placement_policy_util-test.cc@472 PS7, Line 472: for (auto idx = 0; idx < configs.size(); ++idx) { : SCOPED_TRACE(Substitute("test config index: $0", idx)); : const auto& cfg = configs[idx]; : : ClusterLocalityInfo cli; : TabletsPlacementInfo tpi; : ClusterConfigToClusterPlacementInfo(cfg, &cli, &tpi); : : vector<PlacementPolicyViolationInfo> violations; : ASSERT_OK(DetectPlacementPolicyViolations(tpi, &violations)); : NO_FATALS(CheckEqual(cfg.reference_violations_info, violations)); : : vector<Rebalancer::ReplicaMove> moves; : auto s = FindMovesToReimposePlacementPolicy(tpi, cli, violations, &moves); : ASSERT_TRUE(s.IsNotFound()) << s.ToString(); : ASSERT_TRUE(moves.empty()); : } > Couldn't you use RunTest for this? This piece a bit special: it checks for Status::NotFound() from FindMovesToReimposePlacementPolicy() (not from anything else), while RunTest() is using compound-like approach. I would prefer to keep that separate. -- To view, visit http://gerrit.cloudera.org:8080/11549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1cdc3b57a782e6d043b1853600c97fe1f8630347 Gerrit-Change-Number: 11549 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Mon, 29 Oct 2018 20:55:39 +0000 Gerrit-HasComments: Yes
