adelapena commented on code in PR #2460:
URL: https://github.com/apache/cassandra/pull/2460#discussion_r1252243412
##########
src/java/org/apache/cassandra/index/sai/IndexContext.java:
##########
@@ -160,7 +160,7 @@ public IndexMetadata getIndexMetadata()
/**
* @return A set of SSTables which have attached to them invalid index
components.
*/
- public Collection<SSTableContext>
onSSTableChanged(Collection<SSTableReader> oldSSTables,
Collection<SSTableContext> newSSTables, boolean validate)
+ public Collection<SSTableContext>
onSSTableChanged(Collection<SSTableReader> oldSSTables,
Collection<SSTableContext> newSSTables, IndexValidation validate)
Review Comment:
Nit: I'd call the parameter `validation`, instead of the current
boolean-style `validate`
##########
src/java/org/apache/cassandra/index/sai/SSTableContextManager.java:
##########
@@ -52,7 +52,7 @@ public class SSTableContextManager
* @return a set of contexts for SSTables with valid per-SSTable
components, and a set of
* SSTables with invalid or missing components
*/
- public Pair<Set<SSTableContext>, Set<SSTableReader>>
update(Collection<SSTableReader> removed, Iterable<SSTableReader> added,
boolean validate)
+ public Pair<Set<SSTableContext>, Set<SSTableReader>>
update(Collection<SSTableReader> removed, Iterable<SSTableReader> added,
IndexValidation validate)
Review Comment:
I'd call the parameter `validation`. Also, the JavaDoc `@param` above is
outdated.
##########
src/java/org/apache/cassandra/index/sai/IndexContext.java:
##########
@@ -406,7 +406,7 @@ public String logMessage(String message)
* @return the indexes that are built on the given SSTables on the left
and corrupted indexes'
* corresponding contexts on the right
*/
- public Pair<Collection<SSTableIndex>, Collection<SSTableContext>>
getBuiltIndexes(Collection<SSTableContext> sstableContexts, boolean validate)
+ public Pair<Collection<SSTableIndex>, Collection<SSTableContext>>
getBuiltIndexes(Collection<SSTableContext> sstableContexts, IndexValidation
validate)
Review Comment:
Nit: I'd call the parameter `validation`, instead of the current
boolean-style `validate`
##########
src/java/org/apache/cassandra/index/sai/disk/v1/V1OnDiskFormat.java:
##########
@@ -213,6 +211,7 @@ public boolean
validatePerColumnIndexComponents(IndexDescriptor indexDescriptor,
}
}
}
+ long endTime = Clock.Global.currentTimeMillis();
Review Comment:
This seems unused
##########
test/unit/org/apache/cassandra/index/sai/disk/v1/trie/TrieValidationTest.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.index.sai.disk.v1.trie;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.cassandra.db.marshal.UTF8Type;
+import org.apache.cassandra.index.sai.disk.format.IndexComponent;
+import org.apache.cassandra.index.sai.disk.format.IndexDescriptor;
+import org.apache.cassandra.index.sai.disk.io.IndexOutputWriter;
+import org.apache.cassandra.index.sai.disk.v1.SAICodecUtils;
+import org.apache.cassandra.index.sai.utils.SAIRandomizedTester;
+import org.apache.cassandra.io.tries.IncrementalDeepTrieWriterPageAware;
+import org.apache.cassandra.utils.bytecomparable.ByteComparable;
+import org.apache.cassandra.utils.bytecomparable.ByteSource;
+import org.apache.lucene.store.IndexInput;
+
+import static
org.apache.cassandra.index.sai.disk.v1.trie.TrieTermsDictionaryReader.trieSerializer;
+
+public class TrieValidationTest extends SAIRandomizedTester
+{
+ IndexDescriptor indexDescriptor;
+
+ @Before
+ public void createIndexDescriptor() throws Throwable
+ {
+ indexDescriptor = newIndexDescriptor();
+ }
+
+ @Test
+ public void testHeaderValidation() throws Throwable
+ {
+ createSimpleTrie(indexDescriptor);
+ try (IndexInput input =
indexDescriptor.openPerSSTableInput(IndexComponent.PRIMARY_KEY_TRIE))
+ {
+ SAICodecUtils.validate(input);
+ }
+ }
+
+ @Test
+ public void testChecksumValidation() throws Throwable
+ {
+ createSimpleTrie(indexDescriptor);
+ try (IndexInput input =
indexDescriptor.openPerSSTableInput(IndexComponent.PRIMARY_KEY_TRIE))
+ {
+ SAICodecUtils.validateChecksum(input);
+ }
+ }
+
+ private void createSimpleTrie(IndexDescriptor indexDescriptor) throws
Throwable
Review Comment:
Nit: can be `static`
##########
test/unit/org/apache/cassandra/index/sai/disk/v1/trie/TrieValidationTest.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.index.sai.disk.v1.trie;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.cassandra.db.marshal.UTF8Type;
+import org.apache.cassandra.index.sai.disk.format.IndexComponent;
+import org.apache.cassandra.index.sai.disk.format.IndexDescriptor;
+import org.apache.cassandra.index.sai.disk.io.IndexOutputWriter;
+import org.apache.cassandra.index.sai.disk.v1.SAICodecUtils;
+import org.apache.cassandra.index.sai.utils.SAIRandomizedTester;
+import org.apache.cassandra.io.tries.IncrementalDeepTrieWriterPageAware;
+import org.apache.cassandra.utils.bytecomparable.ByteComparable;
+import org.apache.cassandra.utils.bytecomparable.ByteSource;
+import org.apache.lucene.store.IndexInput;
+
+import static
org.apache.cassandra.index.sai.disk.v1.trie.TrieTermsDictionaryReader.trieSerializer;
+
+public class TrieValidationTest extends SAIRandomizedTester
+{
+ IndexDescriptor indexDescriptor;
Review Comment:
Nit: can be `private`
##########
src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java:
##########
@@ -89,60 +89,20 @@ static class IncrementalChecksumSequentialWriter extends
SequentialWriter implem
super(file, writerOption);
}
- @Override
- public void writeByte(int b) throws IOException
- {
- super.writeByte(b);
- checksum.update(b);
- }
-
- @Override
- public void write(byte[] b) throws IOException
- {
- super.write(b);
- checksum.update(b);
- }
-
- @Override
- public void write(byte[] b, int off, int len) throws IOException
- {
- super.write(b, off, len);
- checksum.update(b, off, len);
- }
-
- @Override
- public void writeChar(int v) throws IOException
- {
- super.writeChar(v);
- addTochecksum(v, 2);
- }
-
- @Override
- public void writeInt(int v) throws IOException
- {
- super.writeInt(v);
- addTochecksum(v, 4);
- }
-
- @Override
- public void writeLong(long v) throws IOException
- {
- super.writeLong(v);
- addTochecksum(v, 8);
- }
-
- public long getChecksum()
+ public long getChecksum() throws IOException
Review Comment:
Nit: add `@Override`
##########
test/unit/org/apache/cassandra/index/sai/disk/v1/trie/TrieValidationTest.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.index.sai.disk.v1.trie;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.cassandra.db.marshal.UTF8Type;
+import org.apache.cassandra.index.sai.disk.format.IndexComponent;
+import org.apache.cassandra.index.sai.disk.format.IndexDescriptor;
+import org.apache.cassandra.index.sai.disk.io.IndexOutputWriter;
+import org.apache.cassandra.index.sai.disk.v1.SAICodecUtils;
+import org.apache.cassandra.index.sai.utils.SAIRandomizedTester;
+import org.apache.cassandra.io.tries.IncrementalDeepTrieWriterPageAware;
+import org.apache.cassandra.utils.bytecomparable.ByteComparable;
+import org.apache.cassandra.utils.bytecomparable.ByteSource;
+import org.apache.lucene.store.IndexInput;
+
+import static
org.apache.cassandra.index.sai.disk.v1.trie.TrieTermsDictionaryReader.trieSerializer;
+
+public class TrieValidationTest extends SAIRandomizedTester
+{
+ IndexDescriptor indexDescriptor;
+
+ @Before
+ public void createIndexDescriptor() throws Throwable
+ {
+ indexDescriptor = newIndexDescriptor();
+ }
+
+ @Test
+ public void testHeaderValidation() throws Throwable
+ {
+ createSimpleTrie(indexDescriptor);
+ try (IndexInput input =
indexDescriptor.openPerSSTableInput(IndexComponent.PRIMARY_KEY_TRIE))
+ {
+ SAICodecUtils.validate(input);
+ }
+ }
+
+ @Test
+ public void testChecksumValidation() throws Throwable
+ {
+ createSimpleTrie(indexDescriptor);
+ try (IndexInput input =
indexDescriptor.openPerSSTableInput(IndexComponent.PRIMARY_KEY_TRIE))
+ {
+ SAICodecUtils.validateChecksum(input);
+ }
+ }
+
+ private void createSimpleTrie(IndexDescriptor indexDescriptor) throws
Throwable
+ {
+ try (IndexOutputWriter trieOutput =
indexDescriptor.openPerSSTableOutput(IndexComponent.PRIMARY_KEY_TRIE);
+ IncrementalDeepTrieWriterPageAware<Long> trieWriter = new
IncrementalDeepTrieWriterPageAware<>(trieSerializer,
trieOutput.asSequentialWriter()))
+ {
+ SAICodecUtils.writeHeader(trieOutput);
+ trieWriter.add(v -> createMultiPart(v, "abc", "def", "ghi"), 1L);
+ trieWriter.add(v -> createMultiPart(v, "abc", "def", "jkl"), 2L);
+ trieWriter.add(v -> createMultiPart(v, "abc", "ghi", "jkl"), 3L);
+ trieWriter.add(v -> createMultiPart(v, "def", "ghi", "jkl"), 4L);
+ trieWriter.add(v ->
UTF8Type.instance.asComparableBytes(UTF8Type.instance.fromString("abcdef"), v),
5L);
+ trieWriter.add(v ->
UTF8Type.instance.asComparableBytes(UTF8Type.instance.fromString("abdefg"), v),
6L);
+ trieWriter.add(v ->
UTF8Type.instance.asComparableBytes(UTF8Type.instance.fromString("abdfgh"), v),
7L);
+ trieWriter.complete();
+ SAICodecUtils.writeFooter(trieOutput);
+ }
+ }
+
+ private ByteSource createMultiPart(ByteComparable.Version version,
String... parts)
Review Comment:
Nit: can be `static`
--
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]
