leventov commented on a change in pull request #1696: Add CPU time to metrics
for segment scanning.
URL: https://github.com/apache/incubator-druid/pull/1696#discussion_r227043743
##########
File path: server/src/main/java/io/druid/server/coordination/ServerManager.java
##########
@@ -316,9 +322,15 @@ public void dropSegment(final DataSegment segment) throws
SegmentLoadingExceptio
}
);
- return new FinalizeResultsQueryRunner<T>(
- toolChest.mergeResults(factory.mergeRunners(exec, queryRunners)),
- toolChest
+ return CPUTimeMetricQueryRunner.safeBuild(
+ new FinalizeResultsQueryRunner<T>(
+ toolChest.mergeResults(factory.mergeRunners(exec, queryRunners)),
Review comment:
@drcrallen implementation of most QueryRunnerFactory.mergeRunners() methods
delegates to `ChainedExecutionQueryRunner`, that delegates actual processing to
a provided executor. Hence it seems to me that CPU Time wrapping should happen
before `mergeRunners()`. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
