[GitHub] [pinot] walterddr commented on a diff in pull request #9571: [multi-stage] allow configurable timeout

[GitBox <gi...@apache.org>]

walterddr commented on code in PR #9571:
URL: https://github.com/apache/pinot/pull/9571#discussion_r993864387


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -87,10 +88,18 @@ public MultiStageBrokerRequestHandler(PinotConfiguration config, BrokerRoutingMa
     }
     _reducerHostname = reducerHostname;
     _reducerPort = config.getProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, QueryConfig.DEFAULT_QUERY_RUNNER_PORT);
+    long brokerTimeoutMs = config.getProperty(CommonConstants.Broker.CONFIG_OF_BROKER_TIMEOUT_MS, -1L);
+    if (brokerTimeoutMs == -1) {
+      _timeoutNs = config.getProperty(QueryConfig.KEY_OF_QUERY_RUNNER_TIMEOUT_NANO,
+          QueryConfig.DEFAULT_QUERY_RUNNER_TIMEOUT_NANO);
+    } else {
+      _timeoutNs = brokerTimeoutMs * 1_000_000L;

Review Comment:
   yeah it is a bit hard to keep that tracking. b/c 
   1. the equivalent of the "compile" actually happens on the server. (see the serverRequestUtils, that actually runs the server level compilation and optimizations, which in v1 engine runs on broker)
   2. however, there's the extra step that broker does in v2 that runs the logical plan / optimizer that breaks the query into stages. 
   
   With these 2 points. i think it is fine to keep the timeout as the runtime timeout setting (e.g. exclude the logical plan phase)



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org