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/10/29 05:52:12 UTC

[GitHub] [ozone] ChenSammi opened a new pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

ChenSammi opened a new pull request #2785:
URL: https://github.com/apache/ozone/pull/2785


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


-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] ayushtkn commented on a change in pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#discussion_r739460063



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -312,7 +312,8 @@ private RaftProperties newRaftProperties() {
         StorageUnit.BYTES);
     RaftServerConfigKeys.Log.setQueueElementLimit(
         properties, logQueueNumElements);
-    RaftServerConfigKeys.Log.setQueueByteLimit(properties, logQueueByteLimit);
+    RaftServerConfigKeys.Log.setQueueByteLimit(properties,
+        SizeInBytes.valueOf(logQueueByteLimit));

Review comment:
       How does this prevent integer overflow of ``logQueueByteLimit``, This value is type casted into `(int)` above
   ```
       final int logQueueByteLimit = (int) conf.getStorageSize(
           OzoneConfigKeys.DFS_CONTAINER_RATIS_LOG_QUEUE_BYTE_LIMIT,
           OzoneConfigKeys.DFS_CONTAINER_RATIS_LOG_QUEUE_BYTE_LIMIT_DEFAULT,
           StorageUnit.BYTES);
   ```
   Can you help share some pointers 




-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] adoroszlai merged pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2785:
URL: https://github.com/apache/ozone/pull/2785


   


-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] ChenSammi commented on a change in pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#discussion_r741596983



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -312,7 +312,8 @@ private RaftProperties newRaftProperties() {
         StorageUnit.BYTES);
     RaftServerConfigKeys.Log.setQueueElementLimit(
         properties, logQueueNumElements);
-    RaftServerConfigKeys.Log.setQueueByteLimit(properties, logQueueByteLimit);
+    RaftServerConfigKeys.Log.setQueueByteLimit(properties,
+        SizeInBytes.valueOf(logQueueByteLimit));

Review comment:
       You are right.  This patch is seperated from one of my interval big patch which mixes several fixes.  The cast to (long) line is missed here. 




-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] ChenSammi commented on a change in pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#discussion_r741596983



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -312,7 +312,8 @@ private RaftProperties newRaftProperties() {
         StorageUnit.BYTES);
     RaftServerConfigKeys.Log.setQueueElementLimit(
         properties, logQueueNumElements);
-    RaftServerConfigKeys.Log.setQueueByteLimit(properties, logQueueByteLimit);
+    RaftServerConfigKeys.Log.setQueueByteLimit(properties,
+        SizeInBytes.valueOf(logQueueByteLimit));

Review comment:
       You are right.  This patch is seperated from one of my interval big patch which mixes several fixes.  The cast to (long) line is missed here. 




-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] adoroszlai merged pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2785:
URL: https://github.com/apache/ozone/pull/2785


   


-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] adoroszlai commented on pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#issuecomment-1040191553


   Thanks @ChenSammi for the patch, @ayushtkn for the review.  I guess we can add the unit test later.


-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] ChenSammi commented on a change in pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#discussion_r741596983



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -312,7 +312,8 @@ private RaftProperties newRaftProperties() {
         StorageUnit.BYTES);
     RaftServerConfigKeys.Log.setQueueElementLimit(
         properties, logQueueNumElements);
-    RaftServerConfigKeys.Log.setQueueByteLimit(properties, logQueueByteLimit);
+    RaftServerConfigKeys.Log.setQueueByteLimit(properties,
+        SizeInBytes.valueOf(logQueueByteLimit));

Review comment:
       You are right.  This patch is seperated from one of my interval big patch which mixes several fixes.  The cast to (long) line is missed here. 




-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] ChenSammi commented on a change in pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#discussion_r741596983



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -312,7 +312,8 @@ private RaftProperties newRaftProperties() {
         StorageUnit.BYTES);
     RaftServerConfigKeys.Log.setQueueElementLimit(
         properties, logQueueNumElements);
-    RaftServerConfigKeys.Log.setQueueByteLimit(properties, logQueueByteLimit);
+    RaftServerConfigKeys.Log.setQueueByteLimit(properties,
+        SizeInBytes.valueOf(logQueueByteLimit));

Review comment:
       You are right.  This patch is seperated from one of my interval big patch which mixes several fixes.  The cast to (long) line is missed here. 




-- 
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: issues-unsubscribe@ozone.apache.org

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


[GitHub] [ozone] adoroszlai commented on pull request #2785: HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo…

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2785:
URL: https://github.com/apache/ozone/pull/2785#issuecomment-1040191553


   Thanks @ChenSammi for the patch, @ayushtkn for the review.  I guess we can add the unit test later.


-- 
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: issues-unsubscribe@ozone.apache.org

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