adoroszlai commented on a change in pull request #2202:
URL: https://github.com/apache/ozone/pull/2202#discussion_r639738573
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
##########
@@ -325,6 +327,16 @@ public static int getScmRpcRetryCount(ConfigurationSource
conf) {
OZONE_SCM_HEARTBEAT_RPC_RETRY_COUNT_DEFAULT);
}
+ /**
+ * Get the max limit when get all available reports.
+ * @param conf
+ * @return max limit
+ */
+ public static int getReportMaxLimit(ConfigurationSource conf) {
+ return conf.getInt(HDDS_DATANODE_REPORT_MAX_LIMIT,
+ HDDS_DATANODE_REPORT_MAX_LIMIT_DEFAULT);
+ }
+
Review comment:
Thanks for the answers, and updating the patch.
> 1. In this code, the max reports size of getNonIncrementalReports() return
is 3, if the maxLimit is Integer.MAX_VALUE, so maxLimit < 0 and maxLimit <=
reports.size() is meaningless. I think the maxLimit need to become
configurable.
The checks are not necessarily meaningless, may simply be defensive
programming. Currently the only production caller passes `Integer.MAX_VALUE`.
But the method is public, and new calls may be introduced where different
limits might make sense. In this case they can be kept without making the
limit used by `StateContext` in `getReports()` configurable.
Or they may be unnecessary and could be simply removed.
I was looking for a use case of this configuration from user/administrator
point of view, not based on the code. If we don't know why it may be
configured to different values (i.e. use cases), and how users should come up
with those, then I think it may be better to avoid adding the config.
But that's just my 2 cents.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]