kfaraz commented on code in PR #13463:
URL: https://github.com/apache/druid/pull/13463#discussion_r1047444199
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -615,7 +615,9 @@ public void updatePartialKeyStatisticsInformation(int
stageNumber, int workerNum
queryKernel.addPartialKeyStatisticsForStageAndWorker(stageId,
workerNumber, partialKeyStatisticsInformation);
if
(queryKernel.getStagePhase(stageId).equals(ControllerStagePhase.MERGING_STATISTICS))
{
- List<String> workerTaskIds = workerTaskLauncher.getTaskList();
+ // we only need tasks which are active for this stage.
+ List<String> workerTaskIds = workerTaskLauncher.getTaskList()
Review Comment:
Thanks for the fix, @cryptoe , the new method does seem better. I would also
suggest just doing the filtering of `workerTaskIds` in the controller itself
rather than passing an extra argument to the `WorkerSketchFetcher` just to
filter the task ids later. Then the logic in the `WorkerSketchFetcher` wouldn't
have to change and it wouldn't have to be aware of the worker indexes (which
seems like an impl detail of the controller).
Hope this makes sense.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
