[GitHub] [ignite-3] rpuch commented on a diff in pull request #1170: IGNITE-17832 Fix flaky ItClusterManagerTest#testNodeRestart

[GitBox <gi...@apache.org>]

rpuch commented on code in PR #1170:
URL: https://github.com/apache/ignite-3/pull/1170#discussion_r989790816


##########
modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/AbstractClusterStateStorageTest.java:
##########
@@ -110,6 +110,50 @@ void testPutReplace() {
         assertThat(storage.get(key), is(equalTo(value2)));
     }
 
+    @Test
+    void testReplaceAll() {

Review Comment:
   This test actually contains 3 test cases for 3 properties of the tested method. How about breaking the test method to 3, one per test case?



##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/raft/RocksDbClusterStateStorage.java:
##########
@@ -108,6 +108,22 @@ public void put(byte[] key, byte[] value) {
         }
     }
 
+    @Override
+    public void replaceAll(byte[] prefix, byte[] key, byte[] value) {
+        try (
+                var batch = new WriteBatch();
+                var options = new WriteOptions();
+        ) {
+            batch.deleteRange(prefix, RocksUtils.incrementArray(prefix));

Review Comment:
   It seems that, if `prefix` consists of `0xFF` bytes only, `incrementArray()` will return `null` and `deleteRange()` will fail.
   
   For instance, the prefix is {0xFF}, it contains key {0xFF, 0x5}, and we want to replace it with {0xFF, 0x10}.
   
   Should a scan be added for such unfortunate prefixes?



##########
modules/cluster-management/src/testFixtures/java/org/apache/ignite/internal/cluster/management/raft/TestClusterStateStorage.java:
##########
@@ -62,62 +69,89 @@ public boolean isStarted() {
 
     @Override
     public void put(byte[] key, byte[] value) {
-        map.put(new ByteArray(key), value);
+        lock.readLock().lock();

Review Comment:
   This is a modification operations, should write lock be taken?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org