ashishkumar50 commented on code in PR #4876:
URL: https://github.com/apache/ozone/pull/4876#discussion_r1233580183
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -498,72 +535,138 @@ 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());
+ DataFilter dataFilter = DataFilter.fromValue(missingIn.toUpperCase());
+
+ switch (dataFilter) {
+
+ case SCM:
+ List<Map.Entry<Long, ContainerMetadata>> notSCMContainers =
+ omContainers.entrySet().stream()
+ .filter(
+ containerMetadataEntry ->
!scmNonDeletedContainers.contains(
+ containerMetadataEntry.getKey()))
+ .collect(Collectors.toList());
+
+ 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()));
+ }
- 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);
- });
+ 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);
+ });
+ break;
- // 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());
+ case OM:
+ List<Long> nonOMContainers = scmNonDeletedContainers.stream()
+ .filter(containerId -> !omContainers.containsKey(containerId))
+ .collect(Collectors.toList());
- List<Pipeline> pipelines = new ArrayList<>();
- nonOMContainers.forEach(nonOMContainerId -> {
- ContainerDiscrepancyInfo containerDiscrepancyInfo =
- new ContainerDiscrepancyInfo();
- containerDiscrepancyInfo.setContainerID(nonOMContainerId);
- containerDiscrepancyInfo.setNumberOfKeys(0);
- PipelineID pipelineID = null;
- try {
- pipelineID = containerManager.getContainer(
- ContainerID.valueOf(nonOMContainerId))
- .getPipelineID();
-
- if (null != pipelineID) {
- pipelines.add(pipelineManager.getPipeline(pipelineID));
+ if (prevKey > 0) {
+ int index = 0;
+ while (index < nonOMContainers.size() &&
+ nonOMContainers.get(index) <= prevKey) {
+ index++;
+ }
+ if (index < nonOMContainers.size()) {
+ nonOMContainers = nonOMContainers.subList(index,
+ Math.min(index + limit, nonOMContainers.size()));
+ } else {
+ nonOMContainers = Collections.emptyList();
}
- } catch (ContainerNotFoundException e) {
- LOG.warn("Container {} not found in SCM: {}", nonOMContainerId, e);
- } catch (PipelineNotFoundException e) {
- LOG.debug("Pipeline not found for container: {} and pipelineId: {}",
- nonOMContainerId, pipelineID, e);
+ } else {
+ nonOMContainers = nonOMContainers.subList(0,
+ Math.min(limit, nonOMContainers.size()));
}
- containerDiscrepancyInfo.setPipelines(pipelines);
- containerDiscrepancyInfo.setExistsAt("SCM");
- containerDiscrepancyInfoList.add(containerDiscrepancyInfo);
- });
+ List<Pipeline> pipelines = new ArrayList<>();
+ nonOMContainers.forEach(nonOMContainerId -> {
+ ContainerDiscrepancyInfo containerDiscrepancyInfo =
+ new ContainerDiscrepancyInfo();
+ containerDiscrepancyInfo.setContainerID(nonOMContainerId);
+ containerDiscrepancyInfo.setNumberOfKeys(0);
+ PipelineID pipelineID = null;
+ try {
+ pipelineID = containerManager.getContainer(
+ ContainerID.valueOf(nonOMContainerId)).getPipelineID();
+ if (pipelineID != null) {
+ pipelines.add(pipelineManager.getPipeline(pipelineID));
+ }
+ } catch (ContainerNotFoundException e) {
Review Comment:
Do we need to handle this ContainerNotFoundException, If this happens it
means container is not even in SCM too. As here we are saying container not
found in SCM. But we are marking in L661 as
`containerDiscrepancyInfo.setExistsAt("SCM"); `.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java:
##########
@@ -498,72 +535,138 @@ 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) {
Review Comment:
In PR description mentioned as "If `prevKey` is 0 or negative, the sublist
is retrieved from the start of the list up to the specified limit.". Can you
change the PR description, as in code we are returning `NOT_ACCEPTABLE` which
is correct in case of negative `prevKey`.
--
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]
