alievmirza commented on code in PR #7512:
URL: https://github.com/apache/ignite-3/pull/7512#discussion_r2758842791


##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java:
##########
@@ -2127,6 +2149,25 @@ public void apply(final Task task) {
             throw new IllegalStateException("Node is shutting down");
         }
         Requires.requireNonNull(task, "Null task");
+        Requires.requireNonNull(task.getData(), "Null data");
+
+        int taskDataSize = task.getData().remaining();
+
+        long maxQueueSize = this.raftOptions.getMaxApplyQueueByteSize();
+
+        if (maxQueueSize > 0) {
+            long currentSize = this.applyQueueByteSize.sum();
+            if (currentSize + taskDataSize > maxQueueSize) {

Review Comment:
   Good question. I was thinking about the solution with minimum impact to 
performance. With this solution `maxApplyQueueByteSize` seem to be soft limit, 
not hard, and if we want to make it hard limit, we must introduce some lock/cas 
which definitely will reduce performance.
   
   What do you think about stating in javadoc that `maxApplyQueueByteSize` is 
soft limit and precision is somewhere +- max threads * max log entry size?  



##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java:
##########
@@ -2127,6 +2149,25 @@ public void apply(final Task task) {
             throw new IllegalStateException("Node is shutting down");
         }
         Requires.requireNonNull(task, "Null task");
+        Requires.requireNonNull(task.getData(), "Null data");
+
+        int taskDataSize = task.getData().remaining();
+
+        long maxQueueSize = this.raftOptions.getMaxApplyQueueByteSize();
+
+        if (maxQueueSize > 0) {
+            long currentSize = this.applyQueueByteSize.sum();
+            if (currentSize + taskDataSize > maxQueueSize) {

Review Comment:
   Good question. I was thinking about the solution with the minimum impact to 
performance. With this solution `maxApplyQueueByteSize` seem to be soft limit, 
not hard, and if we want to make it hard limit, we must introduce some lock/cas 
which definitely will reduce performance.
   
   What do you think about stating in javadoc that `maxApplyQueueByteSize` is 
soft limit and precision is somewhere +- max threads * max log entry size?  



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to