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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
