jon-wei commented on a change in pull request #9971:
URL: https://github.com/apache/druid/pull/9971#discussion_r437077302
##########
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:
Added a comment
##########
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:
Changed this to use TemporaryFolder, I also changed
SegmentLoadDropHandlerTest which is where I got the code from
##########
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:
Cleaned up params
##########
File path:
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
##########
@@ -327,6 +327,13 @@ public TaskStatus call()
command.add(nodeType);
}
+ // If the task type is queryable, we need to load
broadcast segments on the peon, used for
+ // join queries
+ if (task.supportsQueries()) {
+ command.add("--loadBroadcastSegments");
Review comment:
Hm, I saw the batch tasks loading things from segment cache, which I was
trying to avoid right now since they could be loaded on-heap for indexed
tables, for performance/memory load reasons.
For the coverage, I'm thinking we should ignore the coverage failure on
ForkingTaskRunner and CliPeon for this PR, does that sound okay to you?
----------------------------------------------------------------
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]
