devmadhuu commented on code in PR #4876:
URL: https://github.com/apache/ozone/pull/4876#discussion_r1227649350
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -498,72 +502,135 @@ private List<ContainerBlockMetadata> getBlocks(
return blockIds;
}
+ /**
+ * Retrieves the container mismatch insights.
+ *
+ * This method returns a list of ContainerDiscrepancyInfo objects
representing
+ * the containers that are missing in either the Ozone Manager (OM) or the
+ * Storage Container Manager (SCM), based on the provided filter parameter.
+ * The returned list is paginated based on the provided limit and prevKey
+ * parameters.
+ *
+ * @param limit The maximum number of container discrepancies to return.
+ * @param prevKey The container ID after which the results are returned.
+ * @param missingIn The missing filter parameter to specify if it's
+ * "OM" or "SCM" missing containers to be returned.
+ */
@GET
@Path("/mismatch")
- public Response getContainerMisMatchInsights() {
+ public Response getContainerMisMatchInsights(
+ @DefaultValue(DEFAULT_FETCH_COUNT)
+ @QueryParam(RECON_QUERY_LIMIT) int limit,
+ @DefaultValue(PREV_CONTAINER_ID_DEFAULT_VALUE)
+ @QueryParam(RECON_QUERY_PREVKEY) long prevKey,
+ @DefaultValue(DEFAULT_FILTER_FOR_MISSING_CONTAINERS)
Review Comment:
Can we make default filter for missing in SCM, which means present in OM,
but missing in SCM. This is more critical (data loss situation)
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -573,9 +640,14 @@ public Response getContainerMisMatchInsights() {
throw new WebApplicationException(ex,
Response.Status.INTERNAL_SERVER_ERROR);
}
- return Response.ok(containerDiscrepancyInfoList).build();
+ Map<String, Object> response = new HashMap<>();
+ response.put("prevKey", prevKey);
+ response.put("missingContainerList", containerDiscrepancyInfoList);
Review Comment:
Don't name it as "missingContainerList". We can keep as
"containerDiscrepancyInfo" only
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -498,72 +502,135 @@ private List<ContainerBlockMetadata> getBlocks(
return blockIds;
}
+ /**
+ * Retrieves the container mismatch insights.
+ *
+ * This method returns a list of ContainerDiscrepancyInfo objects
representing
+ * the containers that are missing in either the Ozone Manager (OM) or the
+ * Storage Container Manager (SCM), based on the provided filter parameter.
+ * The returned list is paginated based on the provided limit and prevKey
+ * parameters.
+ *
+ * @param limit The maximum number of container discrepancies to return.
+ * @param prevKey The container ID after which the results are returned.
+ * @param missingIn The missing filter parameter to specify if it's
+ * "OM" or "SCM" missing containers to be returned.
+ */
@GET
@Path("/mismatch")
- public Response getContainerMisMatchInsights() {
+ public Response getContainerMisMatchInsights(
+ @DefaultValue(DEFAULT_FETCH_COUNT)
+ @QueryParam(RECON_QUERY_LIMIT) int limit,
+ @DefaultValue(PREV_CONTAINER_ID_DEFAULT_VALUE)
+ @QueryParam(RECON_QUERY_PREVKEY) long prevKey,
+ @DefaultValue(DEFAULT_FILTER_FOR_MISSING_CONTAINERS)
+ @QueryParam(RECON_QUERY_FILTER) String missingIn) {
+ if (prevKey < 0 || limit < 0) {
+ // Send back an empty response
+ return Response.status(Response.Status.NOT_ACCEPTABLE).build();
+ }
+
List<ContainerDiscrepancyInfo> containerDiscrepancyInfoList =
new ArrayList<>();
try {
Map<Long, ContainerMetadata> omContainers =
reconContainerMetadataManager.getContainers(-1, -1);
List<Long> scmNonDeletedContainers =
containerManager.getContainers().stream()
- .filter(containerInfo -> !(containerInfo.getState() ==
- HddsProtos.LifeCycleState.DELETED))
- .map(containerInfo -> containerInfo.getContainerID()).collect(
- Collectors.toList());
-
- // Filter list of container Ids which are present in OM but not in SCM.
- List<Map.Entry<Long, ContainerMetadata>> notSCMContainers =
- omContainers.entrySet().stream().filter(containerMetadataEntry ->
- !(scmNonDeletedContainers.contains(
- containerMetadataEntry.getKey())))
- .collect(
- Collectors.toList());
+ .filter(containerInfo -> containerInfo.getState() !=
+ HddsProtos.LifeCycleState.DELETED)
+ .map(containerInfo -> containerInfo.getContainerID())
+ .collect(Collectors.toList());
- notSCMContainers.forEach(nonSCMContainer -> {
- ContainerDiscrepancyInfo containerDiscrepancyInfo =
- new ContainerDiscrepancyInfo();
- containerDiscrepancyInfo.setContainerID(nonSCMContainer.getKey());
- containerDiscrepancyInfo.setNumberOfKeys(
- nonSCMContainer.getValue().getNumberOfKeys());
- containerDiscrepancyInfo.setPipelines(nonSCMContainer.getValue()
- .getPipelines());
- containerDiscrepancyInfo.setExistsAt("OM");
- containerDiscrepancyInfoList.add(containerDiscrepancyInfo);
- });
+ // Filter list of container IDs which are missing in SCM
+ if ("SCM".equalsIgnoreCase(missingIn)) {
+ List<Map.Entry<Long, ContainerMetadata>> notSCMContainers =
+ omContainers.entrySet().stream()
+ .filter(
+ containerMetadataEntry ->
!scmNonDeletedContainers.contains(
+ containerMetadataEntry.getKey()))
+ .collect(Collectors.toList());
+
+ // Filter containers based on pagination parameters
+ if (prevKey > 0) {
+ int index = 0;
+ while (index < notSCMContainers.size() &&
+ notSCMContainers.get(index).getKey() <= prevKey) {
+ index++;
+ }
+ if (index < notSCMContainers.size()) {
+ notSCMContainers = notSCMContainers.subList(index,
+ Math.min(index + limit, notSCMContainers.size()));
+ } else {
+ notSCMContainers = Collections.emptyList();
+ }
+ } else {
+ notSCMContainers = notSCMContainers.subList(0,
+ Math.min(limit, notSCMContainers.size()));
+ }
- // Filter list of container Ids which are present in SCM but not in OM.
- List<Long> nonOMContainers = scmNonDeletedContainers.stream()
- .filter(containerId -> !omContainers.containsKey(containerId))
- .collect(Collectors.toList());
+ notSCMContainers.forEach(nonSCMContainer -> {
+ ContainerDiscrepancyInfo containerDiscrepancyInfo =
+ new ContainerDiscrepancyInfo();
+ containerDiscrepancyInfo.setContainerID(nonSCMContainer.getKey());
+ containerDiscrepancyInfo.setNumberOfKeys(
+ nonSCMContainer.getValue().getNumberOfKeys());
+ containerDiscrepancyInfo.setPipelines(
+ nonSCMContainer.getValue().getPipelines());
+ containerDiscrepancyInfo.setExistsAt("OM");
+ containerDiscrepancyInfoList.add(containerDiscrepancyInfo);
+ });
+ } else if ("OM".equalsIgnoreCase(missingIn)) {
Review Comment:
You can use "`DEFAULT_FILTER_FOR_MISSING_CONTAINERS`" constant to compare
against.
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java:
##########
@@ -1268,16 +1269,74 @@ public void testGetContainerInsightsNonSCMContainers()
// delete container Id 1 from SCM
reconContainerManager.deleteContainer(ContainerID.valueOf(1));
Response containerInsights =
- containerEndpoint.getContainerMisMatchInsights();
+ containerEndpoint.getContainerMisMatchInsights(10, 0, "SCM");
+ Map<String, Object> response =
+ (Map<String, Object>) containerInsights.getEntity();
+ long prevKey = (long) response.get("prevKey");
+ assertEquals(0, prevKey);
List<ContainerDiscrepancyInfo> containerDiscrepancyInfoList =
- (List<ContainerDiscrepancyInfo>) containerInsights.getEntity();
+ (List<ContainerDiscrepancyInfo>) response.get(
+ "missingContainerList");
ContainerDiscrepancyInfo containerDiscrepancyInfo =
containerDiscrepancyInfoList.get(0);
assertEquals(1, containerDiscrepancyInfo.getContainerID());
assertEquals(1, containerDiscrepancyInfoList.size());
assertEquals("OM", containerDiscrepancyInfo.getExistsAt());
}
+
+ @Test
+ public void testGetContainerInsightsNonSCMContainersWithPrevKey()
Review Comment:
Pls check the test case based on above prevKey logic comment.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -573,9 +640,14 @@ public Response getContainerMisMatchInsights() {
throw new WebApplicationException(ex,
Response.Status.INTERNAL_SERVER_ERROR);
}
- return Response.ok(containerDiscrepancyInfoList).build();
+ Map<String, Object> response = new HashMap<>();
+ response.put("prevKey", prevKey);
Review Comment:
I think in "prevKey", value should not be `prevKey` variable value. This
should be the last containerId being returned from list over which prevKey
being iterated.
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java:
##########
@@ -1296,16 +1355,113 @@ public void testGetContainerInsightsNonOMContainers()
}
});
Response containerInsights =
- containerEndpoint.getContainerMisMatchInsights();
+ containerEndpoint.getContainerMisMatchInsights(10, 0, "OM");
+ Map<String, Object> response =
+ (Map<String, Object>) containerInsights.getEntity();
List<ContainerDiscrepancyInfo> containerDiscrepancyInfoList =
- (List<ContainerDiscrepancyInfo>) containerInsights.getEntity();
+ (List<ContainerDiscrepancyInfo>) response.get("missingContainerList");
ContainerDiscrepancyInfo containerDiscrepancyInfo =
containerDiscrepancyInfoList.get(0);
assertEquals(2, containerDiscrepancyInfo.getContainerID());
assertEquals(1, containerDiscrepancyInfoList.size());
assertEquals("SCM", containerDiscrepancyInfo.getExistsAt());
}
+ @Test
+ public void testGetContainerInsightsNonOMContainersWithPrevKey()
Review Comment:
Pls recheck the prevKey logic based on comment.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
