You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:35:46 UTC

[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2303: HDDS-5283. getStorageSize cast to int can cause issue

bharatviswa504 commented on a change in pull request #2303:
URL: https://github.com/apache/ozone/pull/2303#discussion_r649878637



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -164,13 +164,14 @@ public ContainerStateMachine(RaftGroupId gid, ContainerDispatcher dispatcher,
     int numPendingRequests = conf
         .getObject(DatanodeRatisServerConfig.class)
         .getLeaderNumPendingRequests();
-    int pendingRequestsByteLimit = (int) conf.getStorageSize(
+    int pendingRequestsMegaBytesLimit = (int) conf.getStorageSize(
         OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT,
         OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT_DEFAULT,
-        StorageUnit.BYTES);
+        StorageUnit.MB);
     stateMachineDataCache = new ResourceLimitCache<>(new ConcurrentHashMap<>(),
-        (index, data) -> new int[] {1, data.size()}, numPendingRequests,
-        pendingRequestsByteLimit);
+        (index, data) -> new int[] {1,
+            (int) StorageUnit.MB.fromBytes(data.size()) }, numPendingRequests,

Review comment:
       Question if data.size() is in bytes, this conversion will yield to zero? And what locks we acquire for the bytes limit?

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -437,12 +438,13 @@ private RpcType setRpcType(RaftProperties properties) {
 
   private void setPendingRequestsLimits(RaftProperties properties) {
 
-    final int pendingRequestsByteLimit = (int)conf.getStorageSize(
+    final int pendingRequestsMegaBytesLimit = (int)conf.getStorageSize(
         OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT,
         OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT_DEFAULT,
-        StorageUnit.BYTES);
+        StorageUnit.MB);

Review comment:
       Does this need Ratis Change also, like how you are converting Data size to MB which you are doing in StateMachine?
   
   because in ratis 
   
   ```
   tryAcquire(1, Message.getSize(message)
   ```
   




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org