HDDS-8701. Recon - Improve Mismatched container info API (containers/v1/mismatch).

[ArafatKhan2198]

What changes were proposed in this pull request?

In the modified code, the pagination and filtering logic is applied to : getContainerMisMatchInsights()

For the notSCMContainers list (OM containers not in SCM):

• If prevKey is greater than 0, the method finds the index of the next container after prevKey and retrieves the sublist from that index up to the specified limit.
• If prevKey is negative, we return NOT_ACCEPTABLE

For the nonOMContainers list (SCM containers not in OM):

• Similar to the previous section, the pagination logic is applied to retrieve the sublist of containers based on the prevKey and limit.

Filter Parameter:

• The missingIn filter parameter is used in the getContainerMisMatchInsights method to specify whether the returned container discrepancies should be based on containers missing in the Ozone Manager (OM) or the Storage Container Manager (SCM).

Response Type:

• The return type of the method has been changed to Response containing a map. The map includes two entries: prevKey and missingContainerList.

Final Response

{
  "lastKey": <containerID>,
  "containerDiscrepancyInfo": [
    {
      "containerId": <containerID>,
      "numberOfKeys": <numberOfKeys>,
      "pipelines": [
        {
          // Pipeline details
        },
        ...
      ],
      "existsAt": "<existsAt>"
    },
    ...
  ]
}

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8701

How was this patch tested?

Manual Testing and Unit Testing

[adoroszlai]

@ArafatKhan2198 there are checkstyle violations, please check

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
 70: Using the '.*' form of import should be avoided - java.util.*.
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java
 103: Using the '.*' form of import should be avoided - org.junit.Assert.*.

https://github.com/ArafatKhan2198/ozone/actions/runs/5241259025/jobs/9463120676#step:6:529

[ArafatKhan2198]

@devmadhuu @sumitagrawl @dombizita Can you please look !

[ArafatKhan2198]

@devmadhuu @sumitagrawl review changes have been made!

[sumitagrawl]

@ArafatKhan2198 thanks for working over this, given few comments.

[sumitagrawl]

@ArafatKhan2198 Thanks for working, LGTM +1

[devmadhuu]

@ArafatKhan2198 Thanks for working. LGTM +1

[ashishkumar50]

@ArafatKhan2198, Thanks for working on this, overall PR LGTM. Left two comments.

[dombizita]

thanks for working on this @ArafatKhan2198, overall it looks good to me, please address the comments by @ashishkumar50, those are good catches.
also I just saw you renamed the prevKey to lastKey, may I ask why? prevKey is used often in the Recon code, also in your patch still at several places. I'd go with prevKey, the constants are also named in that way.

[ArafatKhan2198]

thanks for working on this @ArafatKhan2198, overall it looks good to me, please address the comments by @ashishkumar50, those are good catches.
also I just saw you renamed the prevKey to lastKey, may I ask why? prevKey is used often in the Recon code, also in your patch still at several places. I'd go with prevKey, the constants are also named in that way.

Thank you for reviewing the code and providing your feedback. Regarding the renaming of prevKey to lastKey, I understand your concern. However, please note that the lastKey you mentioned is not used as a query parameter in the endpoint. It is used as part of the JSON response to implement pagination.

When the getContainerMisMatchInsights method is called, the result is returned, and the last container ID is included in the JSON response as lastKey. This lastKey is not related to the query parameter used in the API endpoint.

Subsequently, when the next set of results is fetched by calling getContainerMisMatchInsights again, the prevKey query parameter will be set as the lastKey response parameter from the previous response, so as to facilitate pagination.
And putting lastKey in the response makes more sense as it will contain the container ID of the last record fetched by the API call.

[ashishkumar50]

@ArafatKhan2198, Thanks for updating patch, LGTM +1.

[sumitagrawl]

@ArafatKhan2198 LGTM +1

[dombizita]

thanks for working on this @ArafatKhan2198! thanks for the review @ashishkumar50, @devmadhuu and @sumitagrawl!