-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add sys.queries table. #18923
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
base: master
Are you sure you want to change the base?
Add sys.queries table. #18923
Conversation
The sys.queries table provides insight into currently-running queries. It provides the same information as the /druid/v2/sql/queries API. As such, it currently only works with Dart. In this patch the table is documented, but off by default. It can be enabled by setting druid.sql.planner.enableSysQueriesTable = true. This patch additionally adds an "includeComplete" parameter to /druid/v2/sql/queries, which is used by the implementation of the sys.queries table, to allow it to show information for recently-completed queries.
multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/ControllerHolder.java
Dismissed
Show dismissed
Hide dismissed
capistrant
left a comment
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.
Cool stuff! I think the bot report re redundant toString is accurate and could be cleaned up. I also left note in the docs to make sure we aren't missing any disclaimer about why this is off by default, if there is such a reason beyond it being experimental.
| * @param roleName the role name to create and assign | ||
| * @param permissions the permissions to grant to the role | ||
| */ | ||
| public static void createUserWithPermissions( |
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.
nice
| ``` | ||
| ``` | ||
|
|
||
| ### QUERIES table |
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.
With this being off by default, I guess it would make me think as an operator that there is either a cost or a risk to turning it on. I don't think risk, but cost, maybe? Or is this just how we have operated with new sys tables in past, feature flag them, and once non-experimental remove the flag?
If there is a consideration an operator should take beyond "this is off by default cuz it is experimental" then I think we should add such a disclaimer 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.
I was thinking it's because it only works with Dart, and Dart is experimental, so this should be experimental too by transitivity. I will add that to the docs.
| { | ||
| // Create mock SqlEngine that returns test queries | ||
| final SqlEngine mockEngine = EasyMock.createMock(SqlEngine.class); | ||
| EasyMock.expect(mockEngine.name()).andReturn("native").anyTimes(); |
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.
just showing the native engine some love by using it in the mocks so it isn't feeling left out by not being in the real thing?
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.
Maybe it'd support native one day 😄
The
sys.queriestable provides insight into currently-running queries. It provides the same information as the/druid/v2/sql/queriesAPI. As such, it currently only works with Dart.In this patch the table is documented, but off by default. It can be enabled by setting
druid.sql.planner.enableSysQueriesTable = true.This patch additionally adds an
includeCompleteparameter to/druid/v2/sql/queries, which is used by the implementation of the sys.queries table, to allow it to show information for recently-completed queries.