You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/09/23 10:31:20 UTC
[GitHub] [druid] clintropolis opened a new pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
clintropolis opened a new pull request #10422:
URL: https://github.com/apache/druid/pull/10422
### Description
Adds a light weight verison of `/druid/coordinator/v1/lookups/nodeStatus`, which currently include the lookup spec JSON, which can be sort of large for inline map lookups:
```json
{
"__default": {
"localhost:8083": {
"current": {
"lookup_table": {
"version": "2020-06-11T10:48:17.033Z",
"lookupExtractorFactory": {
"type": "cachedNamespace",
"extractionNamespace": {
"type": "uri",
"uri": null,
"uriPrefix": "file:/Users/clint/workspace/data/druid/lookups/lookup-simple.json",
"fileRegex": null,
"namespaceParseSpec": {
"format": "simpleJson"
},
"pollPeriod": "PT0S"
},
"firstCacheTimeout": 0,
"injective": true
}
}
},
"toLoad": {},
"toDrop": []
}
}
}
```
Using `/druid/coordinator/v1/lookups/nodeStatus?detailed=false` will instead just include the 'version' string instead of the complete lookup JSON spec:
```json
{
"__default": {
"localhost:8083": {
"current": {
"lookup_table": "2020-06-11T10:48:17.033Z"
},
"toLoad": {},
"toDrop": []
}
}
}
```
The new parameter defaults to true if not set to preserve existing behavior.
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added documentation for new or modified features or behaviors.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
----------------------------------------------------------------
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
[GitHub] [druid] vogievetsky commented on pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #10422:
URL: https://github.com/apache/druid/pull/10422#issuecomment-698001536
I reviewed and 👍 the high level logic and the docs (not the Java)
----------------------------------------------------------------
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #10422:
URL: https://github.com/apache/druid/pull/10422#discussion_r493501074
##########
File path: server/src/main/java/org/apache/druid/server/http/LookupCoordinatorResource.java
##########
@@ -540,9 +542,12 @@ LookupStatus getLookupStatus(
@Produces({MediaType.APPLICATION_JSON})
@Path("/nodeStatus")
public Response getAllNodesStatus(
Review comment:
another nit: does it make sense to break this function into smaller functions ?
----------------------------------------------------------------
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
[GitHub] [druid] asdf2014 merged pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
asdf2014 merged pull request #10422:
URL: https://github.com/apache/druid/pull/10422
----------------------------------------------------------------
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
[GitHub] [druid] vogievetsky commented on pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #10422:
URL: https://github.com/apache/druid/pull/10422#issuecomment-697498493
This came out of a discussion we had about surfacing the info from that API in the web console
----------------------------------------------------------------
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
[GitHub] [druid] asdf2014 merged pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
asdf2014 merged pull request #10422:
URL: https://github.com/apache/druid/pull/10422
----------------------------------------------------------------
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #10422: add light weight version of /druid/coordinator/v1/lookups/nodeStatus
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #10422:
URL: https://github.com/apache/druid/pull/10422#discussion_r493497951
##########
File path: docs/querying/lookups.md
##########
@@ -347,7 +347,7 @@ These end points can be used to get the propagation status of configured lookups
### List lookup state of all processes
-`GET /druid/coordinator/v1/lookups/nodeStatus` with optional query parameter `discover` to discover tiers or configured lookup tiers are listed.
+`GET /druid/coordinator/v1/lookups/nodeStatus` with optional query parameter `discover` to discover tiers or configured lookup tiers are listed. The default response will include the which lookups are loaded, being loaded, or being dropped on each node for each tier, including the complete lookup spec. Add the optional query parameter `detailed=false` to only include the 'version' of the lookup instead of the complete spec.
Review comment:
nit:
```suggestion
`GET /druid/coordinator/v1/lookups/nodeStatus` with optional query parameter `discover` to discover tiers or configured lookup tiers are listed. The response will include the lookups which are loaded, being loaded, or being dropped on each node for each tier. By default, complete lookup spec is included in response for each lookup. Add the optional query parameter `detailed=false` to only include the 'version' of the lookup instead of the complete spec.
```
----------------------------------------------------------------
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