tkalkirill commented on code in PR #4465: URL: https://github.com/apache/ignite-3/pull/4465#discussion_r1778499597
########## modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/AbstractCompactionKeyValueStorageTest.java: ########## @@ -17,186 +17,150 @@ package org.apache.ignite.internal.metastorage.server; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.ignite.internal.metastorage.dsl.Operations.noop; +import static org.apache.ignite.internal.metastorage.dsl.Operations.ops; +import static org.apache.ignite.internal.metastorage.dsl.Operations.put; +import static org.apache.ignite.internal.metastorage.dsl.Operations.remove; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; 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.lang.ByteArray; import org.apache.ignite.internal.metastorage.Entry; +import org.apache.ignite.internal.metastorage.impl.CommandIdGenerator; +import org.apache.ignite.internal.metastorage.server.ExistenceCondition.Type; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** Compaction tests. */ public abstract class AbstractCompactionKeyValueStorageTest extends AbstractKeyValueStorageTest { - private final HybridClock clock = new HybridClockImpl(); - - @Test - public void testCompactionAfterLastRevision() { - byte[] key = key(0); - byte[] value1 = keyValue(0, 0); - byte[] value2 = keyValue(0, 1); + private static final byte[] FOO_KEY = fromString("foo"); - storage.put(key, value1, clock.now()); - storage.put(key, value2, clock.now()); + private static final byte[] BAR_KEY = fromString("bar"); - long lastRevision = storage.revision(); + private static final byte[] SOME_KEY = fromString("someKey"); - storage.compact(clock.now()); + private static final byte[] SOME_VALUE = fromString("someValue"); - // Latest value, must exist. - Entry entry2 = storage.get(key, lastRevision); - assertEquals(lastRevision, entry2.revision()); - assertArrayEquals(value2, entry2.value()); + private final HybridClock clock = new HybridClockImpl(); - // Previous value, must be removed due to compaction. - Entry entry1 = storage.get(key, lastRevision - 1); - assertTrue(entry1.empty()); - } + @Override + @BeforeEach + void setUp() { + super.setUp(); - @Test - public void testCompactionAfterTombstone() { - byte[] key = key(0); - byte[] value = keyValue(0, 0); + storage.putAll(List.of(FOO_KEY, BAR_KEY), List.of(SOME_VALUE, SOME_VALUE), clock.now()); + storage.put(BAR_KEY, SOME_VALUE, clock.now()); + storage.put(FOO_KEY, SOME_VALUE, clock.now()); + storage.put(SOME_KEY, SOME_VALUE, clock.now()); - storage.put(key, value, clock.now()); - storage.remove(key, clock.now()); + var fooKey = new ByteArray(FOO_KEY); + var barKey = new ByteArray(BAR_KEY); - long lastRevision = storage.revision(); + var iif = new If( + new AndCondition(new ExistenceCondition(Type.EXISTS, FOO_KEY), new ExistenceCondition(Type.EXISTS, BAR_KEY)), + new Statement(ops(put(fooKey, SOME_VALUE), remove(barKey)).yield()), + new Statement(ops(noop()).yield()) + ); - storage.compact(clock.now()); + storage.invoke(iif, clock.now(), new CommandIdGenerator(() -> UUID.randomUUID().toString()).newId()); - // Current value, must be removed due to being a tombstone. - Entry entry2 = storage.get(key, lastRevision); - assertTrue(entry2.empty()); + storage.remove(SOME_KEY, clock.now()); - // Previous value, must be removed due to compaction. - Entry entry1 = storage.get(key, lastRevision - 1); - assertTrue(entry1.empty()); + assertEquals(List.of(1, 3, 5), collectRevisions(FOO_KEY)); + assertEquals(List.of(1, 2, 5), collectRevisions(BAR_KEY)); + assertEquals(List.of(4, 6), collectRevisions(SOME_KEY)); } @Test - public void testCompactionBetweenMultipleWrites() { - 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 compactTs = clock.now(); - - storage.put(key, value3, clock.now()); - storage.put(key, value4, clock.now()); - - long lastRevision = storage.revision(); + void testCompactRevision1() { + storage.compact(1); - storage.compact(compactTs); - - Entry entry4 = storage.get(key, lastRevision); - assertArrayEquals(value4, entry4.value()); - - Entry entry3 = storage.get(key, lastRevision - 1); - assertArrayEquals(value3, entry3.value()); - - Entry entry2 = storage.get(key, lastRevision - 2); - assertArrayEquals(value2, entry2.value()); - - // Previous value, must be removed due to compaction. - Entry entry1 = storage.get(key, lastRevision - 3); - assertTrue(entry1.empty()); + assertEquals(List.of(3, 5), collectRevisions(FOO_KEY)); + assertEquals(List.of(2, 5), collectRevisions(BAR_KEY)); + assertEquals(List.of(4, 6), collectRevisions(SOME_KEY)); } @Test - public void testCompactionAfterTombstoneRemovesTombstone() { - byte[] key = key(0); - byte[] value1 = keyValue(0, 0); - byte[] value2 = keyValue(0, 1); - - storage.put(key, value1, clock.now()); - - storage.remove(key, clock.now()); - - HybridTimestamp compactTs = clock.now(); - - storage.put(key, value2, clock.now()); - - storage.remove(key, clock.now()); + void testCompactRevision2() { + storage.compact(2); - long lastRevision = storage.revision(); - - storage.compact(compactTs); - - // Last operation was remove, so this is a tombstone. - Entry entry4 = storage.get(key, lastRevision); - assertTrue(entry4.tombstone()); - - Entry entry3 = storage.get(key, lastRevision - 1); - assertArrayEquals(value2, entry3.value()); - - // Previous value, must be removed due to compaction. - Entry entry2 = storage.get(key, lastRevision - 2); - assertTrue(entry2.empty()); - - Entry entry1 = storage.get(key, lastRevision - 3); - assertTrue(entry1.empty()); + assertEquals(List.of(3, 5), collectRevisions(FOO_KEY)); + assertEquals(List.of(5), collectRevisions(BAR_KEY)); + assertEquals(List.of(4, 6), collectRevisions(SOME_KEY)); } @Test - public void testCompactEmptyStorage() { - storage.compact(clock.now()); + void testCompactRevision3() { + storage.compact(3); + + assertEquals(List.of(5), collectRevisions(FOO_KEY)); + assertEquals(List.of(5), collectRevisions(BAR_KEY)); + assertEquals(List.of(4, 6), collectRevisions(SOME_KEY)); } @Test - public void testCompactionBetweenRevisionsOfOneKey() { - byte[] key = key(0); - byte[] value11 = keyValue(0, 0); - byte[] value12 = keyValue(0, 1); - - storage.put(key, value11, clock.now()); - - byte[] key2 = key(1); - byte[] value2 = keyValue(1, 0); - storage.put(key2, value2, clock.now()); + void testCompactRevision4() { + storage.compact(4); - HybridTimestamp compactTs = clock.now(); + assertEquals(List.of(5), collectRevisions(FOO_KEY)); + assertEquals(List.of(5), collectRevisions(BAR_KEY)); + assertEquals(List.of(6), collectRevisions(SOME_KEY)); + } - storage.put(key, value12, clock.now()); + @Test + void testCompactRevision5() { + storage.compact(5); - storage.compact(compactTs); + assertEquals(List.of(5), collectRevisions(FOO_KEY)); + assertEquals(List.of(), collectRevisions(BAR_KEY)); + assertEquals(List.of(6), collectRevisions(SOME_KEY)); + } - // Both keys should exist, as low watermark's revision is higher than entry11's, but lesser than entry12's, - // this means that entry1 is still needed. - Entry entry12 = storage.get(key, storage.revision()); - assertArrayEquals(value12, entry12.value()); + @Test + void testCompactRevision6() { + storage.compact(6); - Entry entry11 = storage.get(key, storage.revision() - 1); - assertArrayEquals(value11, entry11.value()); + assertEquals(List.of(5), collectRevisions(FOO_KEY)); + assertEquals(List.of(), collectRevisions(BAR_KEY)); + assertEquals(List.of(), collectRevisions(SOME_KEY)); + } - Entry entry2 = storage.get(key2, storage.revision()); - assertArrayEquals(value2, entry2.value()); + @Test + void testCompactRevisionSequentially() { + testCompactRevision1(); + testCompactRevision2(); + testCompactRevision3(); + testCompactRevision4(); + testCompactRevision5(); + testCompactRevision6(); } @Test - public void testInvokeCompactionBeforeAnyEntry() { - byte[] key = key(0); - byte[] value1 = keyValue(0, 0); - byte[] value2 = keyValue(0, 1); + public void testCompactEmptyStorage() { Review Comment: Damn it, you're right, I'll fix it! -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org