jihoonson commented on a change in pull request #9971:
URL: https://github.com/apache/druid/pull/9971#discussion_r436304307
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java
##########
@@ -112,6 +115,9 @@ public DruidCoordinatorRuntimeParams
run(DruidCoordinatorRuntimeParams params)
if (rule.appliesTo(segment, now)) {
stats.accumulate(rule.run(coordinator, paramsWithReplicationManager,
segment));
foundMatchingRule = true;
+ if (rule instanceof BroadcastDistributionRule) {
+ broadcastDatasources.add(segment.getDataSource());
Review comment:
Would you add some comment about that this duty should run before
`BalanceSegments`
[here](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java#L612)?
##########
File path:
server/src/test/java/org/apache/druid/server/coordination/ZkCoordinatorTest.java
##########
@@ -67,9 +76,32 @@ public String getBase()
};
private ZkCoordinator zkCoordinator;
+ private File infoDir;
Review comment:
Please use `TemporaryFolder` of JUnit (an example is found
[here](https://github.com/apache/druid/blob/master/core/src/test/java/org/apache/druid/data/input/impl/LocalInputSourceTest.java#L49-L50)).
This rule automatically cleans up the temp directory on exit.
##########
File path:
server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java
##########
@@ -44,15 +43,15 @@
public static List<Object[]> constructorFeeder()
{
return Lists.newArrayList(
- new Object[]{new
ForeverBroadcastDistributionRule(ImmutableList.of("large_source1",
"large_source2"))},
- new Object[]{new ForeverBroadcastDistributionRule(ImmutableList.of())},
- new Object[]{new ForeverBroadcastDistributionRule(null)},
- new Object[]{new
IntervalBroadcastDistributionRule(Intervals.of("0/1000"),
ImmutableList.of("large_source"))},
- new Object[]{new
IntervalBroadcastDistributionRule(Intervals.of("0/1000"), ImmutableList.of())},
- new Object[]{new
IntervalBroadcastDistributionRule(Intervals.of("0/1000"), null)},
- new Object[]{new PeriodBroadcastDistributionRule(new Period(1000),
null, ImmutableList.of("large_source"))},
- new Object[]{new PeriodBroadcastDistributionRule(new Period(1000),
null, ImmutableList.of())},
- new Object[]{new PeriodBroadcastDistributionRule(new Period(1000),
null, null)}
+ new Object[]{new ForeverBroadcastDistributionRule()},
+ new Object[]{new ForeverBroadcastDistributionRule()},
+ new Object[]{new ForeverBroadcastDistributionRule()},
Review comment:
These seem duplicate. Could you clean up these parameters? Same for the
other below param sets.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
