gortiz commented on code in PR #14574:
URL: https://github.com/apache/pinot/pull/14574#discussion_r1888387562
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -224,67 +227,89 @@ protected BrokerResponse handleRequest(long requestId,
String query, SqlNodeAndO
return new
BrokerResponseNative(QueryException.getException(QueryException.QUOTA_EXCEEDED_ERROR,
errorMessage));
}
- Tracing.ThreadAccountantOps.setupRunner(String.valueOf(requestId),
ThreadExecutionContext.TaskType.MSE);
-
- long executionStartTimeNs = System.nanoTime();
- QueryDispatcher.QueryResult queryResults;
+ Timer queryTimer = new Timer(queryTimeoutMs);
try {
- queryResults =
- _queryDispatcher.submitAndReduce(requestContext,
dispatchableSubPlan, queryTimeoutMs, queryOptions);
- } catch (TimeoutException e) {
- for (String table : tableNames) {
- _brokerMetrics.addMeteredTableValue(table,
BrokerMeter.BROKER_RESPONSES_WITH_TIMEOUTS, 1);
+ // It's fine to block in this thread because we use a separate thread
pool from the main Jersey server to process
+ // these requests.
+ if (!_querySemaphore.tryAcquire(queryTimeoutMs, TimeUnit.MILLISECONDS)) {
+ LOGGER.warn("Timed out waiting to execute request {}: {}", requestId,
query);
+
requestContext.setErrorCode(QueryException.EXECUTION_TIMEOUT_ERROR_CODE);
+ return new
BrokerResponseNative(QueryException.EXECUTION_TIMEOUT_ERROR);
}
- LOGGER.warn("Timed out executing request {}: {}", requestId, query);
+ } catch (InterruptedException e) {
+ LOGGER.warn("Interrupt received while waiting to execute request {}:
{}", requestId, query);
requestContext.setErrorCode(QueryException.EXECUTION_TIMEOUT_ERROR_CODE);
return new BrokerResponseNative(QueryException.EXECUTION_TIMEOUT_ERROR);
Review Comment:
a metric would be great, a log could be useful. But I was more worried about
immediate the feedback to the user. I mean, the error information we return.
But it was a wishful thinking more than an actual requirement.
--
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]
