-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add task report fields in response of SQL statements endpoint #16808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4ea80bb
b611a6b
7f864cb
afa0c96
6a1fc4e
2774447
2320e84
daa425e
174df9a
eda21bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,4 +108,14 @@ private void putAll(final Map<Integer, Map<Integer, CounterSnapshots>> otherMap) | |
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need this now right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's needed for the new test added in the PR: if (actual.getCounters() == null || expected.getCounters() == null) {
Assert.assertEquals(expected.getCounters(), actual.getCounters());
} else {
Assert.assertEquals(expected.getCounters().toString(), actual.getCounters().toString());
} |
||
| { | ||
| synchronized (snapshotsMap) { | ||
| return "CounterSnapshotsTree{" + | ||
| "snapshotsMap=" + snapshotsMap + | ||
| '}'; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,6 +113,7 @@ | |
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.function.Supplier; | ||
| import java.util.stream.Collectors; | ||
|
|
||
|
|
||
|
|
@@ -231,7 +232,9 @@ public Response doPost(final SqlQuery sqlQuery, @Context final HttpServletReques | |
| @Path("/{id}") | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public Response doGetStatus( | ||
| @PathParam("id") final String queryId, @Context final HttpServletRequest req | ||
| @PathParam("id") final String queryId, | ||
| @QueryParam("detail") boolean detail, | ||
|
Akshat-Jain marked this conversation as resolved.
|
||
| @Context final HttpServletRequest req | ||
| ) | ||
| { | ||
| try { | ||
|
|
@@ -242,7 +245,8 @@ public Response doGetStatus( | |
| queryId, | ||
| authenticationResult, | ||
| true, | ||
| Action.READ | ||
| Action.READ, | ||
| detail | ||
| ); | ||
|
|
||
| if (sqlStatementResult.isPresent()) { | ||
|
|
@@ -369,7 +373,8 @@ public Response deleteQuery(@PathParam("id") final String queryId, @Context fina | |
| queryId, | ||
| authenticationResult, | ||
| false, | ||
| Action.WRITE | ||
| Action.WRITE, | ||
| false | ||
| ); | ||
| if (sqlStatementResult.isPresent()) { | ||
| switch (sqlStatementResult.get().getState()) { | ||
|
|
@@ -479,7 +484,7 @@ private Response buildTaskResponse(Sequence<Object[]> sequence, AuthenticationRe | |
| } | ||
| String taskId = String.valueOf(firstRow[0]); | ||
|
|
||
| Optional<SqlStatementResult> statementResult = getStatementStatus(taskId, authenticationResult, true, Action.READ); | ||
| Optional<SqlStatementResult> statementResult = getStatementStatus(taskId, authenticationResult, true, Action.READ, false); | ||
|
|
||
| if (statementResult.isPresent()) { | ||
| return Response.status(Response.Status.OK).entity(statementResult.get()).build(); | ||
|
|
@@ -565,7 +570,8 @@ private Optional<SqlStatementResult> getStatementStatus( | |
| String queryId, | ||
| AuthenticationResult authenticationResult, | ||
| boolean withResults, | ||
| Action forAction | ||
| Action forAction, | ||
| boolean detail | ||
| ) throws DruidException | ||
| { | ||
| TaskStatusResponse taskResponse = contactOverlord(overlordClient.taskStatus(queryId), queryId); | ||
|
|
@@ -582,14 +588,29 @@ private Optional<SqlStatementResult> getStatementStatus( | |
| MSQControllerTask msqControllerTask = getMSQControllerTaskAndCheckPermission(queryId, authenticationResult, forAction); | ||
| SqlStatementState sqlStatementState = SqlStatementResourceHelper.getSqlStatementState(statusPlus); | ||
|
|
||
| Supplier<Optional<MSQTaskReportPayload>> msqTaskReportPayloadSupplier = () -> { | ||
| try { | ||
| return Optional.ofNullable(SqlStatementResourceHelper.getPayload( | ||
| contactOverlord(overlordClient.taskReportAsMap(queryId), queryId) | ||
| )); | ||
| } | ||
| catch (DruidException e) { | ||
| if (e.getErrorCode().equals("notFound")) { | ||
| return Optional.empty(); | ||
|
Akshat-Jain marked this conversation as resolved.
|
||
| } | ||
| throw e; | ||
| } | ||
| }; | ||
|
|
||
| if (SqlStatementState.FAILED == sqlStatementState) { | ||
| return SqlStatementResourceHelper.getExceptionPayload( | ||
| queryId, | ||
| taskResponse, | ||
| statusPlus, | ||
| sqlStatementState, | ||
| contactOverlord(overlordClient.taskReportAsMap(queryId), queryId), | ||
| jsonMapper | ||
| msqTaskReportPayloadSupplier.get().orElse(null), | ||
| jsonMapper, | ||
| detail | ||
| ); | ||
| } else { | ||
| Optional<List<ColumnNameAndTypes>> signature = SqlStatementResourceHelper.getSignature(msqControllerTask); | ||
|
|
@@ -605,7 +626,10 @@ private Optional<SqlStatementResult> getStatementStatus( | |
| sqlStatementState, | ||
| msqControllerTask.getQuerySpec().getDestination() | ||
| ).orElse(null) : null, | ||
| null | ||
| null, | ||
| detail ? SqlStatementResourceHelper.getQueryStagesReport(msqTaskReportPayloadSupplier.get().orElse(null)) : null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wont you call the overlord 3 times here ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| detail ? SqlStatementResourceHelper.getQueryCounters(msqTaskReportPayloadSupplier.get().orElse(null)) : null, | ||
| detail ? SqlStatementResourceHelper.getQueryWarningDetails(msqTaskReportPayloadSupplier.get().orElse(null)) : null | ||
| )); | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.