HDDS-4791. StateContext.getReports may return list with size larger t…

[symious]

…he maxLimit

What changes were proposed in this pull request?

• Fix the logic to return the correct size of the result list.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit Test.

[symious]

@elek @adoroszlai Could you help to review this PR? Thanks in advance.

[dineshchitlangia]

+1 @symious thanks for the improvement

[adoroszlai]

Thanks @symious for finding this discrepancy introduced in HDDS-4404.

The proposed solution has a problem (IllegalArgumentException) in edge case: if maxLimit is smaller than the number of available misc. reports. And if they are equal, getIncrementalReports is called unnecessarily as it will be limited to 0 items.

Also, I'm not sure behavior for maxLimit < 3 can be well defined other than throwing some exception.

Since getReports is called from only one place, with unlimited list size:

ozone/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java

Line 302 in df38294

return getReports(endpoint, Integer.MAX_VALUE);

I think it would be better to just remove maxLimit parameter.

[smengcl]

Thanks @symious for the patch. Yes this was an oversight.

I have the same concern as @adoroszlai mentioned. maxLimit - nonIncrementalReports.size() < 0 can be a problem.

I agree that we should just remove the maxLimit param from getReports():

ozone/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java

Lines 327 to 330 in 6b67ae0

	public List<GeneratedMessage> getReports(InetSocketAddress endpoint,
	int maxLimit) {
	List<GeneratedMessage> reportsToReturn =
	getIncrementalReports(endpoint, maxLimit);

and put getIncrementalReports(endpoint, Integer.MAX_VALUE - nonIncrementalReports.size()) inside.

[symious]

@adoroszlai @smengcl Thanks for the review, added a commit to handle the edge case.

[smengcl]

+1 on the latest change.

A caveat is if maxLimit <= 3, the getReports may always return the same set of reports because once containerReports/nodeReport/pipelineReports are set, they are only updated but not removed. It makes most sense if the caller of getReports specifies maxLimit >= 4.

[symious]

@smengcl @adoroszlai Thanks for the review.