-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add task report fields in response of SQL statements endpoint #16808
Add task report fields in response of SQL statements endpoint #16808
Conversation
06d8aa1
to
6a1fc4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to have detail
as the parameter name or detailed
. detailed=true
seems better than detail=true
. Maybe I am overthinking, but perhaps someone with more knowledge of API designing would help out here.
...core/multi-stage-query/src/main/java/org/apache/druid/msq/counters/CounterSnapshotsTree.java
Outdated
Show resolved
Hide resolved
...core/multi-stage-query/src/main/java/org/apache/druid/msq/counters/CounterSnapshotsTree.java
Show resolved
Hide resolved
...multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
Show resolved
Hide resolved
...re/multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
Outdated
Show resolved
Hide resolved
...multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
Show resolved
Hide resolved
…ated condition" This reverts commit daa425e.
@@ -108,4 +108,14 @@ private void putAll(final Map<Integer, Map<Integer, CounterSnapshots>> otherMap) | |||
} | |||
} | |||
} | |||
|
|||
@Override | |||
public String toString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this now right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed for the new test added in the PR: testMSQSelectRunningQueryWithDetail
as assertSqlStatementResult()
method has the following assertion logic for counters:
if (actual.getCounters() == null || expected.getCounters() == null) {
Assert.assertEquals(expected.getCounters(), actual.getCounters());
} else {
Assert.assertEquals(expected.getCounters().toString(), actual.getCounters().toString());
}
@Akshat-Jain Please add the release notes in the description and relevant documentation for this flag. |
@@ -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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wont you call the overlord 3 times here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…#16808) If the optional query parameter detail is supplied, then the response also includes the following: * A stages object that summarizes information about the different stages being used for query execution, such as stage number, phase, start time, duration, input and output information, processing methods, and partitioning. * A counters object that provides details on the rows, bytes, and files processed at various stages for each worker across different channels, along with sort progress. * A warnings object that provides details about any warnings.
Description
This PR adds an optional query param
detail
to/v2/sql/statements/query-id
endpoint.As per offline discussion with @vogievetsky, the PR adds 3 additional fields to the API response when
detail=true
is passed:stages
: Stages for the query from MSQ task reportcounters
: Stage counters for the query from MSQ task reportwarnings
: Warning reports for the query from MSQ task reportThis is to allow us to have a single API that returns the status and task report for MSQ queries.
Sample API response when detail=true is passed
Release Notes
Add optional boolean query parameter
detail
for the API to get query status.Path:
/v2/sql/statements/query-id?detail=true
Method:
GET
If the optional query parameter
detail
is supplied, then the response also includes the following:stages
object that summarizes information about the different stages being used for query execution, such as stage number, phase, start time, duration, input and output information, processing methods, and partitioning.counters
object that provides details on the rows, bytes, and files processed at various stages for each worker across different channels, along with sort progress.warnings
object that provides details about any warnings.This PR has: