Jackie-Jiang commented on code in PR #15208:
URL: https://github.com/apache/pinot/pull/15208#discussion_r1984124825


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -142,10 +142,19 @@ public void start() {
   public QueryResult submitAndReduce(RequestContext context, 
DispatchableSubPlan dispatchableSubPlan, long timeoutMs,
       Map<String, String> queryOptions)
       throws Exception {
+    return submitAndReduce(context, dispatchableSubPlan, timeoutMs, 
queryOptions, null);
+  }
+
+  public QueryResult submitAndReduce(RequestContext context, 
DispatchableSubPlan dispatchableSubPlan, long timeoutMs,
+      Map<String, String> queryOptions, Runnable beforeReduce)

Review Comment:
   ```suggestion
         Map<String, String> queryOptions, @Nullable Runnable beforeReduce)
   ```



##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -309,18 +309,19 @@ protected BrokerResponse handleRequest(long requestId, 
String query, SqlNodeAndO
       return new BrokerResponseNative(QueryException.EXECUTION_TIMEOUT_ERROR);
     }
 
-    String clientRequestId = extractClientRequestId(sqlNodeAndOptions);
-    onQueryStart(requestId, clientRequestId, query);
-
     try {
       Tracing.ThreadAccountantOps.setupRunner(String.valueOf(requestId), 
ThreadExecutionContext.TaskType.MSE);
 
       long executionStartTimeNs = System.nanoTime();
       QueryDispatcher.QueryResult queryResults;
+      String clientRequestId = extractClientRequestId(sqlNodeAndOptions);
+      //onQueryStart(requestId, clientRequestId, query);

Review Comment:
   I feel we should call `onQueryStart()` here. Any specific reason why you 
want to call it after submit?



-- 
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]

Reply via email to