dcapwell commented on code in PR #2256: URL: https://github.com/apache/cassandra/pull/2256#discussion_r1163419427
########## test/unit/org/apache/cassandra/journal/IndexTest.java: ########## @@ -0,0 +1,133 @@ +/* + * 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.cassandra.journal; + +import java.io.IOException; +import java.nio.file.Files; + +import org.junit.Test; + +import org.apache.cassandra.io.util.File; +import org.apache.cassandra.utils.TimeUUID; + +import static org.apache.cassandra.utils.TimeUUID.Generator.nextTimeUUID; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class IndexTest +{ + private static final int[] EMPTY = new int[0]; + + @Test + public void testInMemoryIndexBasics() + { + InMemoryIndex<TimeUUID> index = InMemoryIndex.create(TimeUUIDKeySupport.INSTANCE); + + TimeUUID key0 = nextTimeUUID(); + TimeUUID key1 = nextTimeUUID(); + TimeUUID key2 = nextTimeUUID(); + TimeUUID key3 = nextTimeUUID(); + TimeUUID key4 = nextTimeUUID(); + + assertEquals(0, index.lookUp(key0).length); + assertEquals(0, index.lookUp(key1).length); + assertEquals(0, index.lookUp(key2).length); + assertEquals(0, index.lookUp(key3).length); + assertEquals(0, index.lookUp(key4).length); + + int val11 = 1100; + int val21 = 2100; + int val22 = 2200; + int val31 = 3100; + int val32 = 3200; + int val33 = 3300; + + index.update(key1, val11); + index.update(key2, val21); + index.update(key2, val22); + index.update(key3, val31); + index.update(key3, val32); + index.update(key3, val33); + + assertArrayEquals(EMPTY, index.lookUp(key0)); + assertArrayEquals(new int[] { val11 }, index.lookUp(key1)); + assertArrayEquals(new int[] { val21, val22 }, index.lookUp(key2)); + assertArrayEquals(new int[] { val31, val32, val33 }, index.lookUp(key3)); + assertArrayEquals(EMPTY, index.lookUp(key4)); + + assertEquals(key1, index.firstId()); + assertEquals(key3, index.lastId()); + + assertFalse(index.mayContainId(key0, TimeUUIDKeySupport.INSTANCE)); + assertTrue(index.mayContainId(key1, TimeUUIDKeySupport.INSTANCE)); + assertTrue(index.mayContainId(key2, TimeUUIDKeySupport.INSTANCE)); + assertTrue(index.mayContainId(key3, TimeUUIDKeySupport.INSTANCE)); + assertFalse(index.mayContainId(key4, TimeUUIDKeySupport.INSTANCE)); + } + + @Test + public void testInMemoryIndexPersists() throws IOException Review Comment: this test is better as a fuzz test rather than using fixed values. I took a stab at that, and seems OnDisk lost a value? ``` diff --git a/src/java/org/apache/cassandra/journal/Journal.java b/src/java/org/apache/cassandra/journal/Journal.java index a9a19ed2c7..eab96c66cc 100644 --- a/src/java/org/apache/cassandra/journal/Journal.java +++ b/src/java/org/apache/cassandra/journal/Journal.java @@ -43,7 +43,6 @@ import org.apache.cassandra.io.util.DataOutputBuffer; import org.apache.cassandra.io.util.File; import org.apache.cassandra.io.util.PathUtils; import org.apache.cassandra.journal.Segments.ReferencedSegments; -import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.utils.Crc; import org.apache.cassandra.utils.JVMStabilityInspector; diff --git a/test/unit/org/apache/cassandra/journal/IndexTest.java b/test/unit/org/apache/cassandra/journal/IndexTest.java index 819e1f1c1f..5306189b50 100644 --- a/test/unit/org/apache/cassandra/journal/IndexTest.java +++ b/test/unit/org/apache/cassandra/journal/IndexTest.java @@ -19,17 +19,30 @@ package org.apache.cassandra.journal; import java.io.IOException; import java.nio.file.Files; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import org.junit.Test; +import org.agrona.collections.IntHashSet; import org.apache.cassandra.io.util.File; +import org.apache.cassandra.utils.Generators; import org.apache.cassandra.utils.TimeUUID; +import org.quicktheories.core.Gen; +import org.quicktheories.impl.Constraint; import static org.apache.cassandra.utils.TimeUUID.Generator.nextTimeUUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.quicktheories.QuickTheory.qt; public class IndexTest { @@ -130,4 +143,118 @@ public class IndexTest assertFalse(onDisk.mayContainId(key4, TimeUUIDKeySupport.INSTANCE)); } } + + @Test + public void prop() throws IOException + { + Constraint sizeConstraint = Constraint.between(1, 10); + Constraint valueSizeConstraint = Constraint.between(0, 10); + Constraint positionConstraint = Constraint.between(0, Integer.MAX_VALUE); + Gen<TimeUUID> keyGen = Generators.timeUUID(); + Gen<int[]> valueGen = rs -> { + int[] array = new int[(int) rs.next(valueSizeConstraint)]; + IntHashSet uniq = new IntHashSet(); + for (int i = 0; i < array.length; i++) + { + int value = (int) rs.next(positionConstraint); + if (!uniq.add(value)) + value = (int) rs.next(positionConstraint); + array[i] = value; + } + return array; + }; + Gen<Map<TimeUUID, int[]>> gen = rs -> { + int size = (int) rs.next(sizeConstraint); + Map<TimeUUID, int[]> map = Maps.newHashMapWithExpectedSize(size); + for (int i = 0; i < size; i++) + { + TimeUUID key = keyGen.generate(rs); + while (map.containsKey(key)) + key = keyGen.generate(rs); + int[] value = valueGen.generate(rs); + map.put(key, value); + } + return map; + }; + gen = gen.describedAs(map -> { + StringBuilder sb = new StringBuilder(); + for (Map.Entry<TimeUUID, int[]> entry : map.entrySet()) + sb.append('\n').append(entry.getKey()).append('\t').append(Arrays.toString(entry.getValue())); + return sb.toString(); + }); + File directory = new File(Files.createTempDirectory(null)); + directory.deleteOnExit(); + qt().withFixedSeed(1735898489413583L).forAll(gen).checkAssert(map -> test(directory, map)); + } + + @Test + public void sample() throws IOException + { + File directory = new File(Files.createTempDirectory(null)); + directory.deleteOnExit(); + + test(directory, ImmutableMap.of(TimeUUID.fromString("58ed800a-d357-11b2-0000-000000000000"), new int[]{ 0 }, + TimeUUID.fromString("8001c17e-d357-11b2-0000-000000000000"), new int[]{ 0 } + )); + } + + private static void test(File directory, Map<TimeUUID, int[]> map) + { + InMemoryIndex<TimeUUID> inMemory = InMemoryIndex.create(TimeUUIDKeySupport.INSTANCE); + for (Map.Entry<TimeUUID, int[]> e : map.entrySet()) + { + TimeUUID key = e.getKey(); + assertThat(inMemory.lookUp(key)).isEmpty(); + assertThat(inMemory.mayContainId(key, TimeUUIDKeySupport.INSTANCE)).isFalse(); + + int[] value = e.getValue(); + if (value.length == 0) + continue; + for (int i : value) + inMemory.update(key, i); + Arrays.sort(value); + } + assertIndex(map, inMemory); + + Descriptor descriptor = Descriptor.create(directory, System.nanoTime(), 1); + inMemory.persist(descriptor); + + try (OnDiskIndex<TimeUUID> onDisk = OnDiskIndex.open(descriptor, TimeUUIDKeySupport.INSTANCE)) + { + assertIndex(map, onDisk); + } + } + + private static void assertIndex(Map<TimeUUID, int[]> expected, Index<TimeUUID> actual) + { + List<TimeUUID> keys = expected.entrySet().stream().filter(e -> e.getValue().length != 0).map(Map.Entry::getKey).collect(Collectors.toList()); + Collections.sort(keys); + if (keys.isEmpty()) + { + assertThat(actual.firstId()).describedAs("Index %s had wrong firstId", actual).isNull(); + assertThat(actual.lastId()).describedAs("Index %s had wrong lastId", actual).isNull(); + } + else + { + assertThat(actual.firstId()).describedAs("Index %s had wrong firstId", actual).isEqualTo(keys.get(0)); + assertThat(actual.lastId()).describedAs("Index %s had wrong lastId", actual).isEqualTo(keys.get(keys.size() - 1)); + } + + for (Map.Entry<TimeUUID, int[]> e : expected.entrySet()) + { + TimeUUID key = e.getKey(); + int[] value = e.getValue(); + int[] read = actual.lookUp(key); + if (value.length == 0) + { + assertThat(read).describedAs("Index %s returned wrong values for %s", actual, key).isEmpty(); + assertThat(actual.mayContainId(key, TimeUUIDKeySupport.INSTANCE)).describedAs("Index %s did not expect %s to exist", key, actual).isFalse(); + } + else + { + assertThat(read).describedAs("Index %s returned wrong values for %s", actual, key).isEqualTo(value); + assertThat(actual.mayContainId(key, TimeUUIDKeySupport.INSTANCE)).describedAs("Index %s expected %s to exist", key, actual).isTrue(); + } + } + } } \ No newline at end of file diff --git a/test/unit/org/apache/cassandra/utils/Generators.java b/test/unit/org/apache/cassandra/utils/Generators.java index 3d17828426..77408b2a0e 100644 --- a/test/unit/org/apache/cassandra/utils/Generators.java +++ b/test/unit/org/apache/cassandra/utils/Generators.java @@ -407,6 +407,25 @@ public final class Generators .map(s -> new String(s.getBytes(StandardCharsets.UTF_8), StandardCharsets.UTF_8)); } + public static Gen<TimeUUID> timeUUID() + { + ZonedDateTime now = ZonedDateTime.of(2020, 8, 20, + 0, 0, 0, 0, ZoneOffset.UTC); + ZonedDateTime startOfTime = now.minusYears(50); + ZonedDateTime endOfDays = now.plusYears(50); + Constraint micros = Constraint.between(toMicros(startOfTime), toMicros(endOfDays)); + return rs -> { + long nowMicro = rs.next(micros); + long lsb = rs.next(Constraint.none()); + return new TimeUUID(TimeUUID.unixMicrosToRawTimestamp(nowMicro), lsb); + }; + } + + private static long toMicros(ZonedDateTime zdt) + { + return zdt.toInstant().toEpochMilli() * 1000 + zdt.getNano() / 1000; + } + private static boolean isDash(char c) { switch (c) ``` Failure ``` Smallest found falsifying value(s) :- 58ed800a-d357-11b2-0000-000000000000 [0] 8001c17e-d357-11b2-0000-000000000000 [0] 58ed8000-d357-11b2-0000-000000000000 [] Cause was :- org.opentest4j.AssertionFailedError: [Index org.apache.cassandra.journal.OnDiskIndex@338fc1d8 returned wrong values for 8001c17e-d357-11b2-0000-000000000000] Expecting: <[]> to be equal to: <[0]> ... Seed was 1735898489413583 ``` -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
