[GitHub] [druid] clintropolis commented on pull request #10426: more timeout handling in JsonParserIterator

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

clintropolis commented on pull request #10426:
URL: https://github.com/apache/druid/pull/10426#issuecomment-697999289


   >This might not be within the scope of this PR, but do you think it would be useful to separate out timeout exceptions from QueryInterruptedException? We could have something like a QueryTimeoutException being thrown in case of timeouts and QueryResource can handle this separately by returning a 504 status instead of 500. I feel returning the 504 error code would be more useful for cluster operators to differentiate between query timeouts vs other types of failures.
   
   Yeah, out of scope of what I was trying to do here, but it is probably worth discussing if `QueryInterruptedException` should be decomposed into several different exceptions - I don't know the answer but it does come up from time to time. It does a reasonable job of encoding the exception information, but it does feel a bit rough. We could potentially even look for `QueryInterruptedException.QUERY_TIMEOUT` as the error and vary the response status code that way, without splitting out the exceptions, but it might be a bit cleaner if we did.


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

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



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