[GitHub] [hadoop] vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit support to /api/containers and /api/containers/{id} endpoints
vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit
support to /api/containers and /api/containers/{id} endpoints
URL: https://github.com/apache/hadoop/pull/954#discussion_r294545140
##
File path:
hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java
##
@@ -181,6 +182,12 @@ public Integer getCountForForContainerKeyPrefix(
Long containerID = keyValue.getKey().getContainerId();
Integer numberOfKeys = keyValue.getValue();
+ // break the loop if limit has been reached
+ // and one more new entity needs to be added to the containers map
+ if (containers.size() == limit && !containers.containsKey(containerID)) {
Review comment:
Without the second condition, the last container ID will have incorrect
number of keys in its containerMetadata. Even when containers limit is reached,
the next iteration could contain the same container ID with a different key
prefix. To handle this situation, we should not break the iterator until the
second condition is also met.
I don't think we need to add any check for any other negative values since
none of them match the condition `containers.size() == limit` at any time.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit support to /api/containers and /api/containers/{id} endpoints
vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit
support to /api/containers and /api/containers/{id} endpoints
URL: https://github.com/apache/hadoop/pull/954#discussion_r294527046
##
File path:
hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java
##
@@ -72,10 +74,11 @@
* @return {@link Response}
*/
@GET
- public Response getContainers() {
+ public Response getContainers(
+ @DefaultValue("-1") @QueryParam("limit") int limit) {
Map containersMap;
try {
- containersMap = containerDBServiceProvider.getContainers();
+ containersMap = containerDBServiceProvider.getContainers(limit);
Review comment:
Yes, that's correct.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit support to /api/containers and /api/containers/{id} endpoints
vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit
support to /api/containers and /api/containers/{id} endpoints
URL: https://github.com/apache/hadoop/pull/954#discussion_r293602190
##
File path:
hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java
##
@@ -72,10 +74,11 @@
* @return {@link Response}
*/
@GET
- public Response getContainers() {
+ public Response getContainers(
+ @DefaultValue("-1") @QueryParam("limit") int limit) {
Map containersMap;
try {
- containersMap = containerDBServiceProvider.getContainers();
+ containersMap = containerDBServiceProvider.getContainers(limit);
} catch (IOException ioEx) {
Review comment:
Adding total count to response and supporting "start" query param is tracked
via https://issues.apache.org/jira/browse/HDDS-1685 and will be implemented
soon.
This PR only adds "limit" support to the APIs.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit support to /api/containers and /api/containers/{id} endpoints
vivekratnavel commented on a change in pull request #954: HDDS-1670. Add limit
support to /api/containers and /api/containers/{id} endpoints
URL: https://github.com/apache/hadoop/pull/954#discussion_r293530930
##
File path:
hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java
##
@@ -72,10 +74,11 @@
* @return {@link Response}
*/
@GET
- public Response getContainers() {
+ public Response getContainers(
+ @DefaultValue("-1") @QueryParam("limit") int limit) {
Map containersMap;
try {
- containersMap = containerDBServiceProvider.getContainers();
+ containersMap = containerDBServiceProvider.getContainers(limit);
} catch (IOException ioEx) {
Review comment:
@bharatviswa504 This PR only supports limit param and I don't see the need
for skip param support in the near feature. I will explain how the UI will
consume this API to show containers and keys to the users. UI will fetch this
API with an initial limit of 50 (arbitrary number or could be x% of
totalCount). When the user scrolls to the end of the list, UI will trigger
another call to the same API with limit + 50 to get 100 items. In a similar
fashion, UI will keep loading results with infinite scroll like something
similar to this demo - https://infinite-scroll.com/demo/full-page/ . Since, the
results are not going to be paginated, there is no need for skip param support
here in my opinion.
The only thing missing is the totalCount in the response of these APIs and
that will be implemented as part of another JIRA if needed in the future.
Infinite scroll could be implemented without total count but having total count
in the UI will give better user experience.
Please let me know if you have any more questions.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
