gianm commented on code in PR #20293:
URL: https://github.com/apache/druid/pull/20293#discussion_r3975647836
##########
multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerHolder.java:
##########
@@ -169,7 +181,15 @@ public ListenableFuture<?> runAsync(
Thread.currentThread().setName(makeThreadName());
try {
- final CaptureReportQueryListener reportListener = new
CaptureReportQueryListener(listener);
+ final CaptureReportQueryListener reportListener = new
CaptureReportQueryListener(listener)
+ {
+ @Override
+ public void onQueryComplete(final MSQTaskReportPayload report)
+ {
+ finalReport = TaskReport.buildTaskReports(new
MSQTaskReport(controller.queryId(), report));
Review Comment:
It seems sad that we need to capture the report into a volatile in two
levels: both here and in `CaptureReportQueryListener`. How about instead
modifying `ControllerImpl#liveReports()` to return the final report when it's
available? Then we should be able to revert `ControllerHolder#getReports` back
to just doing `controller.liveReports()`. We may also be able to remove
`CaptureReportQueryListener` completely, since callers could get the final
report directly out of the `Controller`.
If this works out, consider renaming `Controller#liveReports` to
`Controller#getReports` since it will now sometimes be "live" and sometimes be
final. Its javadoc should also clarify when it's guaranteed to be final. (After
`onQueryComplete` finished? Immediately before `onQueryComplete` is called?)
--
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]
