tkalkirill commented on code in PR #2019:
URL: https://github.com/apache/ignite-3/pull/2019#discussion_r1185772474
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -60,6 +61,10 @@ public class SimpleInMemoryKeyValueStorage implements
KeyValueStorage {
/** Keys index. Value is the list of all revisions under which entry
corresponding to the key was modified. */
private NavigableMap<byte[], List<Long>> keysIdx = new TreeMap<>(CMP);
+ private NavigableMap<Long, Long> tsToRevMap = new TreeMap<>();
Review Comment:
```suggestion
private final NavigableMap<Long, Long> tsToRevMap = new TreeMap<>();
```
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -60,6 +61,10 @@ public class SimpleInMemoryKeyValueStorage implements
KeyValueStorage {
/** Keys index. Value is the list of all revisions under which entry
corresponding to the key was modified. */
private NavigableMap<byte[], List<Long>> keysIdx = new TreeMap<>(CMP);
+ private NavigableMap<Long, Long> tsToRevMap = new TreeMap<>();
+
+ private NavigableMap<Long, Long> revToTsMap = new TreeMap<>();
Review Comment:
I think we can get rid of him for now.
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryCompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+/** Compaction test for the simple in-memory implementation of {@link
KeyValueStorage}. */
+public class SimpleInMemoryCompactionKeyValueStorageTest extends
CompactionKeyValueStorageTest {
+ /** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/RocksDbCompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import java.nio.file.Path;
+import
org.apache.ignite.internal.metastorage.server.persistence.RocksDbKeyValueStorage;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/** Compaction test for the RocksDB implementation of {@link KeyValueStorage}.
*/
+@ExtendWith(WorkDirectoryExtension.class)
+public class RocksDbCompactionKeyValueStorageTest extends
CompactionKeyValueStorageTest {
+ @WorkDirectory
+ private Path workDir;
+
+ /** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/BaseKeyValueStorageTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+/**
+ * Abstract test for {@link KeyValueStorage}.
+ */
+public abstract class BaseKeyValueStorageTest {
+ protected KeyValueStorage storage;
+
+ /**
+ * Before each.
+ */
+ @BeforeEach
+ public void setUp() {
+ storage = storage();
+
+ storage.start();
+ }
+
+ /**
+ * After each.
+ */
+ @AfterEach
+ void tearDown() throws Exception {
+ storage.close();
+ }
+
+ /**
+ * Returns key value storage for this test.
+ */
+ abstract KeyValueStorage storage();
Review Comment:
```suggestion
abstract KeyValueStorage createStorage();
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
Review Comment:
please rename
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+ byte[] value4 = keyValue(0, 3);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+ HybridTimestamp ts = clock.now();
+ storage.put(key, value3, clock.now());
+ storage.put(key, value4, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(ts);
+
+ // Previous value, must be removed due to compaction.
+ Entry entry4 = storage.get(key, lastRevision);
+ assertArrayEquals(value4, entry4.value());
+ }
+
+ @Test
+ public void test4() {
Review Comment:
please rename
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+ byte[] value4 = keyValue(0, 3);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+ HybridTimestamp ts = clock.now();
+ storage.put(key, value3, clock.now());
+ storage.put(key, value4, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(ts);
+
+ // Previous value, must be removed due to compaction.
+ Entry entry4 = storage.get(key, lastRevision);
+ assertArrayEquals(value4, entry4.value());
Review Comment:
Shouldn't the value3 have remained? `compactTs` time is less than it.
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -60,6 +61,10 @@ public class SimpleInMemoryKeyValueStorage implements
KeyValueStorage {
/** Keys index. Value is the list of all revisions under which entry
corresponding to the key was modified. */
private NavigableMap<byte[], List<Long>> keysIdx = new TreeMap<>(CMP);
+ private NavigableMap<Long, Long> tsToRevMap = new TreeMap<>();
Review Comment:
Missing javadoc.
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -518,23 +528,45 @@ private void compactForKey(
byte[] key,
List<Long> revs,
Map<byte[], List<Long>> compactedKeysIdx,
- Map<Long, NavigableMap<byte[], Value>> compactedRevsIdx
+ Map<Long, NavigableMap<byte[], Value>> compactedRevsIdx,
+ long maxRevision
) {
- Long lastRev = lastRevision(revs);
+ List<Long> revsToKeep = new ArrayList<>();
+
+ boolean firstToKeep = true;
+
+ for (int i = 0; i < revs.size(); i++) {
+ long rev = revs.get(i);
- NavigableMap<byte[], Value> kv = revsIdx.get(lastRev);
+ if (rev > maxRevision || (i == revs.size() - 1)) {
+ // If this revision is higher than max revision or is the last
revision, we may need to keep it.
+ NavigableMap<byte[], Value> kv = revsIdx.get(rev);
- Value lastVal = kv.get(key);
+ Value value = kv.get(key);
- if (!lastVal.tombstone()) {
- compactedKeysIdx.put(key, listOf(lastRev));
+ if (firstToKeep) {
Review Comment:
why should we keep the first one?
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -473,13 +481,15 @@ public void removeWatch(WatchListener listener) {
}
@Override
- public void compact() {
+ public void compact(HybridTimestamp lowWatermark) {
synchronized (mux) {
NavigableMap<byte[], List<Long>> compactedKeysIdx = new
TreeMap<>(CMP);
NavigableMap<Long, NavigableMap<byte[], Value>> compactedRevsIdx =
new TreeMap<>();
- keysIdx.forEach((key, revs) -> compactForKey(key, revs,
compactedKeysIdx, compactedRevsIdx));
+ long maxRevision =
tsToRevMap.floorEntry(lowWatermark.longValue()).getValue();
Review Comment:
It seems that if the map is empty, then there will be an NPE.
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/BaseKeyValueStorageTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+/**
+ * Abstract test for {@link KeyValueStorage}.
+ */
+public abstract class BaseKeyValueStorageTest {
+ protected KeyValueStorage storage;
+
+ /**
+ * Before each.
+ */
Review Comment:
```suggestion
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
Review Comment:
please renemae
##########
modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java:
##########
@@ -518,23 +528,45 @@ private void compactForKey(
byte[] key,
List<Long> revs,
Map<byte[], List<Long>> compactedKeysIdx,
- Map<Long, NavigableMap<byte[], Value>> compactedRevsIdx
+ Map<Long, NavigableMap<byte[], Value>> compactedRevsIdx,
+ long maxRevision
) {
- Long lastRev = lastRevision(revs);
+ List<Long> revsToKeep = new ArrayList<>();
+
+ boolean firstToKeep = true;
+
+ for (int i = 0; i < revs.size(); i++) {
+ long rev = revs.get(i);
- NavigableMap<byte[], Value> kv = revsIdx.get(lastRev);
+ if (rev > maxRevision || (i == revs.size() - 1)) {
Review Comment:
The code below is difficult to read, needs to be rewritten and simplified.
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/BaseKeyValueStorageTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+/**
+ * Abstract test for {@link KeyValueStorage}.
+ */
+public abstract class BaseKeyValueStorageTest {
+ protected KeyValueStorage storage;
+
+ /**
+ * Before each.
+ */
+ @BeforeEach
+ public void setUp() {
+ storage = storage();
+
+ storage.start();
+ }
+
+ /**
+ * After each.
+ */
Review Comment:
```suggestion
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+ byte[] value4 = keyValue(0, 3);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+ HybridTimestamp ts = clock.now();
+ storage.put(key, value3, clock.now());
+ storage.put(key, value4, clock.now());
Review Comment:
```suggestion
storage.put(key, value1, clock.now());
storage.put(key, value2, clock.now());
HybridTimestamp compactTs = clock.now();
storage.put(key, value3, clock.now());
storage.put(key, value4, clock.now());
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
Review Comment:
```suggestion
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
Review Comment:
please rename
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+ byte[] value4 = keyValue(0, 3);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+ HybridTimestamp ts = clock.now();
+ storage.put(key, value3, clock.now());
+ storage.put(key, value4, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(ts);
+
+ // Previous value, must be removed due to compaction.
+ Entry entry4 = storage.get(key, lastRevision);
+ assertArrayEquals(value4, entry4.value());
+ }
+
+ @Test
+ public void test4() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+
+ HybridTimestamp now1 = clock.now();
+ storage.put(key, value1, now1);
+ HybridTimestamp now2 = clock.now();
+ storage.put(key, value2, now2);
+ HybridTimestamp ts = clock.now();
+ HybridTimestamp now3 = clock.now();
+ storage.remove(key, now3);
+ HybridTimestamp now4 = clock.now();
+ storage.put(key, value3, now4);
+ storage.remove(key, clock.now());
Review Comment:
```suggestion
storage.put(key, value1, clock.now());
storage.put(key, value2, clock.now());
HybridTimestamp comactTs = clock.now();
storage.remove(key, clock.now());
storage.put(key, value3, clock.now());
storage.remove(key, clock.now());
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/BaseKeyValueStorageTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+/**
+ * Abstract test for {@link KeyValueStorage}.
+ */
+public abstract class BaseKeyValueStorageTest {
Review Comment:
```suggestion
public abstract class AbstractKeyValueStorageTest {
```
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/CompactionKeyValueStorageTest.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.metastorage.server;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.junit.jupiter.api.Test;
+
+/** Compaction tests. */
+public abstract class CompactionKeyValueStorageTest extends
BaseKeyValueStorageTest {
+
+ private final HybridClock clock = new HybridClockImpl();
+
+ @Test
+ public void test1() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Latest value, must exist.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertEquals(lastRevision, entry2.revision());
+ assertArrayEquals(value2, entry2.value());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test2() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+
+ storage.put(key, value1, clock.now());
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(clock.now());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry2 = storage.get(key, lastRevision);
+ assertTrue(entry2.empty());
+
+ // Previous value, must be removed due to compaction.
+ Entry entry1 = storage.get(key, lastRevision - 1);
+ assertTrue(entry1.empty());
+ }
+
+ @Test
+ public void test3() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+ byte[] value4 = keyValue(0, 3);
+
+ storage.put(key, value1, clock.now());
+ storage.put(key, value2, clock.now());
+ HybridTimestamp ts = clock.now();
+ storage.put(key, value3, clock.now());
+ storage.put(key, value4, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(ts);
+
+ // Previous value, must be removed due to compaction.
+ Entry entry4 = storage.get(key, lastRevision);
+ assertArrayEquals(value4, entry4.value());
+ }
+
+ @Test
+ public void test4() {
+ byte[] key = key(0);
+ byte[] value1 = keyValue(0, 0);
+ byte[] value2 = keyValue(0, 1);
+ byte[] value3 = keyValue(0, 2);
+
+ HybridTimestamp now1 = clock.now();
+ storage.put(key, value1, now1);
+ HybridTimestamp now2 = clock.now();
+ storage.put(key, value2, now2);
+ HybridTimestamp ts = clock.now();
+ HybridTimestamp now3 = clock.now();
+ storage.remove(key, now3);
+ HybridTimestamp now4 = clock.now();
+ storage.put(key, value3, now4);
+ storage.remove(key, clock.now());
+
+ long lastRevision = storage.revision();
+
+ storage.compact(ts);
+
+ // Previous value, must be removed due to compaction.
+ Entry entry4 = storage.get(key, lastRevision - 1);
Review Comment:
Please check other values
--
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]
