kfaraz commented on code in PR #13564:
URL: https://github.com/apache/druid/pull/13564#discussion_r1050333053
##########
sql/src/main/java/org/apache/druid/sql/http/SqlResource.java:
##########
@@ -320,4 +181,210 @@ public Response cancelQuery(
return Response.status(Status.FORBIDDEN).build();
}
}
+
+ /**
+ * The SqlResource only generates metrics and doesn't keep track of
aggregate counts of successful/failed/interrupted
+ * queries, so this implementation is effectively just a noop.
+ */
+ private static class SqlResourceQueryMetricCounter implements
QueryResource.QueryMetricCounter
+ {
+ @Override
+ public void incrementSuccess()
+ {
+
+ }
+
+ @Override
+ public void incrementFailed()
+ {
+
+ }
+
+ @Override
+ public void incrementInterrupted()
+ {
+
+ }
+
+ @Override
+ public void incrementTimedOut()
+ {
+
+ }
+ }
+
+ private class SqlResourceQueryResultPusher extends QueryResultPusher
+ {
+ private final String sqlQueryId;
+ private final HttpStatement stmt;
+ private final SqlQuery sqlQuery;
+
+ public SqlResourceQueryResultPusher(
+ AsyncContext asyncContext,
+ String sqlQueryId,
+ HttpStatement stmt,
+ SqlQuery sqlQuery
+ )
+ {
+ super(
+ (HttpServletResponse) asyncContext.getResponse(),
+ SqlResource.this.jsonMapper,
+ SqlResource.this.responseContextConfig,
+ SqlResource.this.selfNode,
+ SqlResource.QUERY_METRIC_COUNTER,
+ sqlQueryId,
+ MediaType.APPLICATION_JSON_TYPE
+ );
+ this.sqlQueryId = sqlQueryId;
+ this.stmt = stmt;
+ this.sqlQuery = sqlQuery;
+ }
+
+ @Override
+ public ResultsWriter start()
+ {
+ return new ResultsWriter()
+ {
+ private ResultSet thePlan;
+
+ @Override
+ @Nullable
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ public QueryResponse<Object> start(HttpServletResponse response)
+ {
+ response.setHeader(SQL_QUERY_ID_RESPONSE_HEADER, sqlQueryId);
+
+ final QueryResponse<Object[]> retVal;
+ try {
+ thePlan = stmt.plan();
+ retVal = thePlan.run();
+ }
+ catch (RelOptPlanner.CannotPlanException e) {
+ throw new SqlPlanningException(
+ SqlPlanningException.PlanningError.UNSUPPORTED_SQL_ERROR,
+ e.getMessage()
+ );
+ }
+ // There is a claim that Calcite sometimes throws a
java.lang.AssertionError, but we do not have a test that can
+ // reproduce it checked into the code (the best we have is something
that uses mocks to throw an Error, which is
+ // dubious at best). We keep this just in case, but it might be
best to remove it and see where the
+ // AssertionErrors are coming from and do something to ensure that
they don't actually make it out of Calcite
+ catch (AssertionError e) {
+ log.warn(e, "AssertionError killed query: %s", sqlQuery);
+
+ // We wrap the exception here so that we get the sanitization.
java.lang.AssertionError apparently
+ // doesn't implement
org.apache.druid.common.exception.SanitizableException.
+ final QueryInterruptedException wrappedEx = new
QueryInterruptedException(e);
+ recordFailure(wrappedEx);
Review Comment:
Should this exception also be thrown similar to the prev catch block rather
than being recorded here?
--
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]
