alex-plekhanov commented on a change in pull request #9833:
URL: https://github.com/apache/ignite/pull/9833#discussion_r818487906



##########
File path: 
modules/core/src/main/java/org/apache/ignite/internal/util/BasicRateLimiter.java
##########
@@ -136,7 +143,7 @@ public void acquire(int permits) throws 
IgniteInterruptedCheckedException {
      * @param permits The number of permits.
      * @return Time in microseconds to wait until the resource can be 
acquired, never negative.
      */
-    private long reserve(int permits) {
+    private long reserve(long permits) {

Review comment:
       It's only one usage of this method and this usage with `int` parameter. 
So, we should make parameter as long in  `acquire` method (also remove one cast 
from long to int in in `acquire` method usage) or revert this change. 

##########
File path: 
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java
##########
@@ -258,6 +266,19 @@
     /** Prefix for snapshot threads. */
     public static final String SNAPSHOT_RUNNER_THREAD_PREFIX = 
"snapshot-runner";
 
+    /** Snapshot transfer rate distributed configuration key */
+    public static final String SNAPSHOT_TRANSFER_RATE_DMS_KEY = 
"snapshotTransferRate";
+
+    /** Snapshot transfer rate is unlimited by default. */
+    public static final long DFLT_SNAPSHOT_TRANSFER_RATE = 0L;
+
+    /** Minimum block size for limited snapshot transfer. */
+    @SystemProperty(value = "Sets the minimum block size for limited snapshot 
transfer")
+    public static final String SNAPSHOT_LIMITED_TRANSFER_BLOCK_SIZE = 
"SNAPSHOT_LIMITED_TRANSFER_BLOCK_SIZE";

Review comment:
       Property name should be started with "IGNITE_".
   Do we really need to configure this property? Why don't just set some 
reasonable constant value?

##########
File path: 
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java
##########
@@ -2994,6 +2997,44 @@ public void testMasterKeyChangeOnInactiveCluster() 
throws Exception {
         assertContains(log, testOut.toString(), "Master key change was 
rejected. The cluster is inactive.");
     }
 
+    /** @throws Exception If failed. */
+    @Test
+    @WithSystemProperty(key = SNAPSHOT_LIMITED_TRANSFER_BLOCK_SIZE, value = 
"1")

Review comment:
       Since `SNAPSHOT_TRANSFER_BLOCK_SIZE_BYTES` is static, this 
`WithSystemProperty` does nothing (unless this test is the first after JVM 
start).  

##########
File path: 
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java
##########
@@ -258,6 +267,20 @@
     /** Prefix for snapshot threads. */
     public static final String SNAPSHOT_RUNNER_THREAD_PREFIX = 
"snapshot-runner";
 
+    /** Snapshot transfer rate distributed configuration key */
+    public static final String SNAPSHOT_TRANSFER_RATE_DMS_KEY = 
"snapshotTransferRate";
+
+    /** Snapshot transfer rate is unlimited by default. */
+    public static final long DFLT_SNAPSHOT_TRANSFER_RATE_BYTES = 0L;
+
+    /** Minimum block size for limited snapshot transfer. */
+    @SystemProperty(value = "Sets the minimum block size for limited snapshot 
transfer")
+    public static final String SNAPSHOT_LIMITED_TRANSFER_BLOCK_SIZE = 
"SNAPSHOT_LIMITED_TRANSFER_BLOCK_SIZE";
+
+    /** Snapshot limited transfer block size (in bytes). */
+    private static final int SNAPSHOT_TRANSFER_BLOCK_SIZE_BYTES =

Review comment:
       Why it's bounded to IO chunk size? Currently, this property is only used 
for local copy.




-- 
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