abhishekagarwal87 commented on code in PR #13815:
URL: https://github.com/apache/druid/pull/13815#discussion_r1110595391
##########
core/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java:
##########
@@ -191,7 +191,8 @@ public boolean hasNext()
{
final long thisTimeoutNanos = timeoutAtNanos -
System.nanoTime();
if (hasTimeout && thisTimeoutNanos < 0) {
- throw new QueryTimeoutException("Sequence iterator timed
out");
+ throw new QueryTimeoutException("Query did not complete
within configured timeout period. " +
+ "You can increase query timeout or tune the performance
of query");
Review Comment:
it's tricky. I cannot put all the kinds of nuances in the error message. I
intentionally avoided writing the configuration. For one reason, the timeout
can be specified in different places. it can come from context, default
timeout, and default cluster-wide configuration.
this is where the error code may help where we return an error code and ask
users to look up the documentation for more info on that error code.
btw, If I were writing this after your PR (error framework) is merged, how
will this code be written?
##########
core/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java:
##########
@@ -206,7 +207,8 @@ public boolean hasNext()
}
}
if (currentBatch == null) {
- throw new QueryTimeoutException("Sequence iterator timed
out waiting for data");
+ throw new QueryTimeoutException("Query did not complete
within configured timeout period. " +
Review Comment:
👍
--
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]
