This is an automated email from the ASF dual-hosted git repository.

adelapena pushed a commit to branch cep-7-sai
in repository https://gitbox.apache.org/repos/asf/cassandra.git

commit 73dcc94a140d2afcc21ff5fb2a2fe8c62061fe10
Author: Piotr Kołaczkowski <[email protected]>
AuthorDate: Tue Jun 20 16:03:03 2023 +0200

    Various changes to SAI index validation
    
    - Fix checksum calculation in IncrementalChecksumSequentialWriter
    - Checksum per-SSTable and per-column components after streaming
    - Avoid validating indexes when full rebuild is requested
    
    patch by Piotr Kołaczkowski; reviewed by Caleb Rackliffe and Andres de la 
Peña for CASSANDRA-18490
    
    Co-authored-by: Piotr Kołaczkowski <[email protected]>
    Co-authored-by: Caleb Rackliffe <[email protected]>
---
 .../apache/cassandra/index/sai/IndexContext.java   | 10 +--
 .../cassandra/index/sai/IndexValidation.java       | 38 +++++++++
 .../cassandra/index/sai/SSTableContextManager.java |  6 +-
 .../cassandra/index/sai/StorageAttachedIndex.java  | 13 +--
 .../index/sai/StorageAttachedIndexBuilder.java     | 10 ++-
 .../index/sai/StorageAttachedIndexGroup.java       | 16 ++--
 .../index/sai/disk/format/IndexDescriptor.java     | 28 +++----
 .../index/sai/disk/io/IndexFileUtils.java          | 69 +++-------------
 .../index/sai/disk/io/IndexOutputWriter.java       | 12 ++-
 .../index/sai/disk/v1/V1OnDiskFormat.java          | 13 ++-
 .../cassandra/index/sai/view/IndexViewManager.java |  9 ++-
 .../org/apache/cassandra/index/sai/SAITester.java  | 15 ++--
 .../index/sai/disk/v1/trie/TrieValidationTest.java | 93 ++++++++++++++++++++++
 13 files changed, 212 insertions(+), 120 deletions(-)

diff --git a/src/java/org/apache/cassandra/index/sai/IndexContext.java 
b/src/java/org/apache/cassandra/index/sai/IndexContext.java
index f16607395d..14a6db6c4b 100644
--- a/src/java/org/apache/cassandra/index/sai/IndexContext.java
+++ b/src/java/org/apache/cassandra/index/sai/IndexContext.java
@@ -162,9 +162,9 @@ public class IndexContext
     /**
      * @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 validation)
     {
-        return viewManager.update(oldSSTables, newSSTables, validate);
+        return viewManager.update(oldSSTables, newSSTables, validation);
     }
 
     public ColumnMetadata getDefinition()
@@ -408,7 +408,7 @@ public class IndexContext
      * @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 
validation)
     {
         Set<SSTableIndex> valid = new HashSet<>(sstableContexts.size());
         Set<SSTableContext> invalid = new HashSet<>();
@@ -433,9 +433,9 @@ public class IndexContext
 
             try
             {
-                if (validate)
+                if (validation != IndexValidation.NONE)
                 {
-                    if 
(!sstableContext.indexDescriptor.validatePerIndexComponents(this))
+                    if 
(!sstableContext.indexDescriptor.validatePerIndexComponents(this, validation))
                     {
                         logger.warn(logMessage("Invalid per-column component 
for SSTable {}"), sstableContext.descriptor());
                         invalid.add(sstableContext);
diff --git a/src/java/org/apache/cassandra/index/sai/IndexValidation.java 
b/src/java/org/apache/cassandra/index/sai/IndexValidation.java
new file mode 100644
index 0000000000..edd9e0fd1c
--- /dev/null
+++ b/src/java/org/apache/cassandra/index/sai/IndexValidation.java
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+public enum IndexValidation
+{
+    /**
+     * No validation to be performed
+     */
+    NONE,
+
+    /**
+     * Basic header/footer validation, but no data validation (fast)
+     */
+    HEADER_FOOTER,
+
+    /**
+     * Full validation with checksumming data (slow)
+     */
+    CHECKSUM
+
+}
diff --git a/src/java/org/apache/cassandra/index/sai/SSTableContextManager.java 
b/src/java/org/apache/cassandra/index/sai/SSTableContextManager.java
index ba65806e86..85192f525e 100644
--- a/src/java/org/apache/cassandra/index/sai/SSTableContextManager.java
+++ b/src/java/org/apache/cassandra/index/sai/SSTableContextManager.java
@@ -47,12 +47,12 @@ public class SSTableContextManager
      *
      * @param removed SSTables being removed
      * @param added SSTables being added
-     * @param validate if true, header and footer will be validated.
+     * @param validation Controls how indexes should be validated
      *
      * @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 validation)
     {
         release(removed);
 
@@ -77,7 +77,7 @@ public class SSTableContextManager
             try
             {
                 // Only validate on restart or newly refreshed SSTable. Newly 
built files are unlikely to be corrupted.
-                if (validate && !sstableContexts.containsKey(sstable) && 
!indexDescriptor.validatePerSSTableComponents())
+                if (!sstableContexts.containsKey(sstable) && 
!indexDescriptor.validatePerSSTableComponents(validation))
                 {
                     logger.warn(indexDescriptor.logMessage("Invalid 
per-SSTable component for SSTable {}"), sstable.descriptor);
                     invalid.add(sstable);
diff --git a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java 
b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
index 44095ebf7e..c826e1da3a 100644
--- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
+++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java
@@ -271,10 +271,11 @@ public class StorageAttachedIndex implements Index
         // New storage-attached indexes will be available for queries after on 
disk index data are built.
         // Memtable data will be indexed via flushing triggered by schema 
change
         // We only want to validate the index files if we are starting up
-        return () -> startInitialBuild(baseCfs, 
StorageService.instance.isStarting()).get();
+        IndexValidation validation = StorageService.instance.isStarting() ? 
IndexValidation.HEADER_FOOTER : IndexValidation.NONE;
+        return () -> startInitialBuild(baseCfs, validation).get();
     }
 
-    private Future<?> startInitialBuild(ColumnFamilyStore baseCfs, boolean 
validate)
+    private Future<?> startInitialBuild(ColumnFamilyStore baseCfs, 
IndexValidation validation)
     {
         if (baseCfs.indexManager.isIndexQueryable(this))
         {
@@ -303,7 +304,7 @@ public class StorageAttachedIndex implements Index
 
         assert indexGroup != null : "Index group does not exist for table " + 
baseCfs.keyspace + '.' + baseCfs.name;
 
-        List<SSTableReader> nonIndexed = findNonIndexedSSTables(baseCfs, 
indexGroup, validate);
+        List<SSTableReader> nonIndexed = findNonIndexedSSTables(baseCfs, 
indexGroup, validation);
 
         if (nonIndexed.isEmpty())
         {
@@ -430,7 +431,7 @@ public class StorageAttachedIndex implements Index
 
             assert indexGroup != null : "Index group does not exist for table";
 
-            Collection<SSTableReader> nonIndexed = 
findNonIndexedSSTables(baseCfs, indexGroup, true);
+            Collection<SSTableReader> nonIndexed = 
findNonIndexedSSTables(baseCfs, indexGroup, IndexValidation.HEADER_FOOTER);
 
             if (nonIndexed.isEmpty())
             {
@@ -522,13 +523,13 @@ public class StorageAttachedIndex implements Index
      *
      * @return a list SSTables without attached indexes
      */
-    private synchronized List<SSTableReader> 
findNonIndexedSSTables(ColumnFamilyStore baseCfs, StorageAttachedIndexGroup 
group, boolean validate)
+    private synchronized List<SSTableReader> 
findNonIndexedSSTables(ColumnFamilyStore baseCfs, StorageAttachedIndexGroup 
group, IndexValidation validation)
     {
         Set<SSTableReader> sstables = baseCfs.getLiveSSTables();
 
         // Initialize the SSTable indexes w/ valid existing components...
         assert group != null : "Missing index group on " + baseCfs.name;
-        group.onSSTableChanged(Collections.emptyList(), sstables, 
Collections.singleton(this), validate);
+        group.onSSTableChanged(Collections.emptyList(), sstables, 
Collections.singleton(this), validation);
 
         // ...then identify and rebuild the SSTable indexes that are missing.
         List<SSTableReader> nonIndexed = new ArrayList<>();
diff --git 
a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexBuilder.java 
b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexBuilder.java
index 8908b83df0..76240c6094 100644
--- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexBuilder.java
+++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexBuilder.java
@@ -259,10 +259,12 @@ public class StorageAttachedIndexBuilder extends 
SecondaryIndexBuilder
      */
     private CountDownLatch shouldWritePerSSTableFiles(SSTableReader sstable)
     {
-        // if per-table files are incomplete or checksum failed during full 
rebuild.
         IndexDescriptor indexDescriptor = IndexDescriptor.create(sstable);
-        if (!indexDescriptor.isPerSSTableIndexBuildComplete() ||
-            (isFullRebuild && 
!indexDescriptor.validatePerSSTableComponentsChecksum()))
+
+        // if per-table files are incomplete, full rebuild is requested, or 
checksum fails
+        if (!indexDescriptor.isPerSSTableIndexBuildComplete()
+            || isFullRebuild
+            || 
!indexDescriptor.validatePerSSTableComponents(IndexValidation.CHECKSUM))
         {
             CountDownLatch latch = CountDownLatch.newCountDownLatch(1);
             if (inProgress.putIfAbsent(sstable, latch) == null)
@@ -307,7 +309,7 @@ public class StorageAttachedIndexBuilder extends 
SecondaryIndexBuilder
 
         // register custom index components into existing sstables
         
sstable.registerComponents(StorageAttachedIndexGroup.getLiveComponents(sstable, 
existing), tracker);
-        Set<StorageAttachedIndex> incomplete = 
group.onSSTableChanged(Collections.emptyList(), Collections.singleton(sstable), 
existing, false);
+        Set<StorageAttachedIndex> incomplete = 
group.onSSTableChanged(Collections.emptyList(), Collections.singleton(sstable), 
existing, IndexValidation.NONE);
 
         if (!incomplete.isEmpty())
         {
diff --git 
a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java 
b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
index 6d1d49c483..7dec78c4da 100644
--- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
+++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java
@@ -250,7 +250,7 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
             SSTableAddedNotification notice = (SSTableAddedNotification) 
notification;
 
             // Avoid validation for index files just written following 
Memtable flush.
-            boolean validate = !notice.memtable().isPresent();
+            IndexValidation validate = notice.memtable().isPresent() ? 
IndexValidation.NONE : IndexValidation.CHECKSUM;
             onSSTableChanged(Collections.emptySet(), notice.added, indexes, 
validate);
         }
         else if (notification instanceof SSTableListChangedNotification)
@@ -258,7 +258,7 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
             SSTableListChangedNotification notice = 
(SSTableListChangedNotification) notification;
 
             // Avoid validation for index files just written during compaction.
-            onSSTableChanged(notice.removed, notice.added, indexes, false);
+            onSSTableChanged(notice.removed, notice.added, indexes, 
IndexValidation.NONE);
         }
         else if (notification instanceof MemtableRenewedNotification)
         {
@@ -298,9 +298,9 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
      * files being corrupt or being unable to successfully update their views
      */
     synchronized Set<StorageAttachedIndex> 
onSSTableChanged(Collection<SSTableReader> removed, Iterable<SSTableReader> 
added,
-                                                            
Set<StorageAttachedIndex> indexes, boolean validate)
+                                                            
Set<StorageAttachedIndex> indexes, IndexValidation validation)
     {
-        Pair<Set<SSTableContext>, Set<SSTableReader>> results = 
contextManager.update(removed, added, validate);
+        Pair<Set<SSTableContext>, Set<SSTableReader>> results = 
contextManager.update(removed, added, validation);
 
         if (!results.right.isEmpty())
         {
@@ -321,7 +321,7 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
 
         for (StorageAttachedIndex index : indexes)
         {
-            Collection<SSTableContext> invalid = 
index.getIndexContext().onSSTableChanged(removed, results.left, validate);
+            Collection<SSTableContext> invalid = 
index.getIndexContext().onSSTableChanged(removed, results.left, validation);
 
             if (!invalid.isEmpty())
             {
@@ -410,8 +410,8 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
     public void unsafeReload()
     {
         contextManager.clear();
-        onSSTableChanged(baseCfs.getLiveSSTables(), Collections.emptySet(), 
indexes, false);
-        onSSTableChanged(Collections.emptySet(), baseCfs.getLiveSSTables(), 
indexes, true);
+        onSSTableChanged(baseCfs.getLiveSSTables(), Collections.emptySet(), 
indexes, IndexValidation.NONE);
+        onSSTableChanged(Collections.emptySet(), baseCfs.getLiveSSTables(), 
indexes, IndexValidation.HEADER_FOOTER);
     }
 
     /**
@@ -422,6 +422,6 @@ public class StorageAttachedIndexGroup implements 
Index.Group, INotificationCons
     {
         contextManager.clear();
         indexes.forEach(StorageAttachedIndex::makeIndexNonQueryable);
-        onSSTableChanged(baseCfs.getLiveSSTables(), Collections.emptySet(), 
indexes, false);
+        onSSTableChanged(baseCfs.getLiveSSTables(), Collections.emptySet(), 
indexes, IndexValidation.NONE);
     }
 }
diff --git 
a/src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java 
b/src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java
index 7261671eca..5aa2a985d5 100644
--- a/src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java
+++ b/src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java
@@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory;
 import org.apache.cassandra.db.ClusteringComparator;
 import org.apache.cassandra.db.lifecycle.LifecycleNewTracker;
 import org.apache.cassandra.index.sai.IndexContext;
+import org.apache.cassandra.index.sai.IndexValidation;
 import org.apache.cassandra.index.sai.SSTableContext;
 import org.apache.cassandra.index.sai.StorageAttachedIndex;
 import org.apache.cassandra.index.sai.disk.PerColumnIndexWriter;
@@ -327,28 +328,25 @@ public class IndexDescriptor
     }
 
     @SuppressWarnings("BooleanMethodIsAlwaysInverted")
-    public boolean validatePerIndexComponents(IndexContext indexContext)
+    public boolean validatePerIndexComponents(IndexContext indexContext, 
IndexValidation validation)
     {
-        logger.info(indexContext.logMessage("Validating per-column index 
components"));
-        return version.onDiskFormat().validatePerColumnIndexComponents(this, 
indexContext, false);
-    }
+        if (validation == IndexValidation.NONE)
+            return true;
 
-    @VisibleForTesting
-    public boolean validatePerIndexComponentsChecksum(IndexContext 
indexContext)
-    {
-        return version.onDiskFormat().validatePerColumnIndexComponents(this, 
indexContext, true);
+        logger.info(indexContext.logMessage("Validating per-column index 
components using mode " + validation));
+        boolean checksum = validation == IndexValidation.CHECKSUM;
+        return version.onDiskFormat().validatePerColumnIndexComponents(this, 
indexContext, checksum);
     }
 
     @SuppressWarnings("BooleanMethodIsAlwaysInverted")
-    public boolean validatePerSSTableComponents()
+    public boolean validatePerSSTableComponents(IndexValidation validation)
     {
-        return version.onDiskFormat().validatePerSSTableIndexComponents(this, 
false);
-    }
+        if (validation == IndexValidation.NONE)
+            return true;
 
-    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
-    public boolean validatePerSSTableComponentsChecksum()
-    {
-        return version.onDiskFormat().validatePerSSTableIndexComponents(this, 
true);
+        logger.info(logMessage("Validating per-sstable index components using 
mode " + validation));
+        boolean checksum = validation == IndexValidation.CHECKSUM;
+        return version.onDiskFormat().validatePerSSTableIndexComponents(this, 
checksum);
     }
 
     public void deletePerSSTableIndexComponents()
diff --git 
a/src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java 
b/src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java
index 1394d20555..6cf9175247 100644
--- a/src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java
+++ b/src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java
@@ -19,7 +19,7 @@
 package org.apache.cassandra.index.sai.disk.io;
 
 import java.io.IOException;
-import java.nio.ByteOrder;
+import java.nio.ByteBuffer;
 import java.util.zip.CRC32;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -58,7 +58,7 @@ public class IndexFileUtils
     {
         assert writerOption.finishOnClose() : "IndexOutputWriter relies on 
close() to sync with disk.";
 
-        return new IndexOutputWriter(new 
IncrementalChecksumSequentialWriter(file, writerOption));
+        return new IndexOutputWriter(new ChecksummingWriter(file, 
writerOption));
     }
 
     public IndexInput openInput(FileHandle handle)
@@ -75,74 +75,27 @@ public class IndexFileUtils
         return IndexInputReader.create(randomReader, fileHandle::close);
     }
 
-    public interface ChecksumWriter
-    {
-        long getChecksum();
-    }
-
-    static class IncrementalChecksumSequentialWriter extends SequentialWriter 
implements ChecksumWriter
+    static class ChecksummingWriter extends SequentialWriter
     {
         private final CRC32 checksum = new CRC32();
 
-        IncrementalChecksumSequentialWriter(File file, SequentialWriterOption 
writerOption)
+        ChecksummingWriter(File file, SequentialWriterOption writerOption)
         {
             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
         {
+            flush();
             return checksum.getValue();
         }
 
-        private void addTochecksum(long bytes, int count)
+        @Override
+        protected void flushData()
         {
-            int origCount = count;
-            if (ByteOrder.BIG_ENDIAN == buffer.order())
-                while (count > 0) checksum.update((int) (bytes >>> (8 * 
--count)));
-            else
-                while (count > 0) checksum.update((int) (bytes >>> (8 * 
(origCount - count--))));
+            ByteBuffer toAppend = buffer.duplicate().flip();
+            super.flushData();
+            checksum.update(toAppend);
         }
     }
 }
diff --git 
a/src/java/org/apache/cassandra/index/sai/disk/io/IndexOutputWriter.java 
b/src/java/org/apache/cassandra/index/sai/disk/io/IndexOutputWriter.java
index 29c4e60625..4e801011da 100644
--- a/src/java/org/apache/cassandra/index/sai/disk/io/IndexOutputWriter.java
+++ b/src/java/org/apache/cassandra/index/sai/disk/io/IndexOutputWriter.java
@@ -62,9 +62,9 @@ public class IndexOutputWriter extends IndexOutput
     }
 
     @Override
-    public long getChecksum()
+    public long getChecksum() throws IOException
     {
-        return ((IndexFileUtils.ChecksumWriter)out).getChecksum();
+        return ((IndexFileUtils.ChecksummingWriter)out).getChecksum();
     }
 
     @Override
@@ -106,10 +106,16 @@ public class IndexOutputWriter extends IndexOutput
     @Override
     public String toString()
     {
+        String checksum;
+        try {
+            checksum = String.valueOf(getChecksum());
+        } catch (IOException e) {
+            checksum = "unknown due to I/O error: " + e;
+        }
         return MoreObjects.toStringHelper(this)
                           .add("path", out.getPath())
                           .add("bytesWritten", getFilePointer())
-                          .add("crc", getChecksum())
+                          .add("crc", checksum)
                           .toString();
     }
 
diff --git 
a/src/java/org/apache/cassandra/index/sai/disk/v1/V1OnDiskFormat.java 
b/src/java/org/apache/cassandra/index/sai/disk/v1/V1OnDiskFormat.java
index 830712b45f..763e1af102 100644
--- a/src/java/org/apache/cassandra/index/sai/disk/v1/V1OnDiskFormat.java
+++ b/src/java/org/apache/cassandra/index/sai/disk/v1/V1OnDiskFormat.java
@@ -176,14 +176,11 @@ public class V1OnDiskFormat implements OnDiskFormat
                 }
                 catch (Throwable e)
                 {
-                    if (logger.isDebugEnabled())
-                    {
-                        logger.debug(indexDescriptor.logMessage("{} failed for 
index component {} on SSTable {}. Error: {}"),
-                                     checksum ? "Checksum validation" : 
"Validation",
-                                     indexComponent,
-                                     indexDescriptor.sstableDescriptor,
-                                     e);
-                    }
+                    logger.error(indexDescriptor.logMessage("{} failed for 
index component {} on SSTable {}. Error: {}"),
+                                 checksum ? "Checksum validation" : 
"Validation",
+                                 indexComponent,
+                                 indexDescriptor.sstableDescriptor,
+                                 e);
                     return false;
                 }
             }
diff --git a/src/java/org/apache/cassandra/index/sai/view/IndexViewManager.java 
b/src/java/org/apache/cassandra/index/sai/view/IndexViewManager.java
index e49230dcb9..72912d52eb 100644
--- a/src/java/org/apache/cassandra/index/sai/view/IndexViewManager.java
+++ b/src/java/org/apache/cassandra/index/sai/view/IndexViewManager.java
@@ -29,6 +29,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.index.sai.IndexContext;
+import org.apache.cassandra.index.sai.IndexValidation;
 import org.apache.cassandra.index.sai.SSTableContext;
 import org.apache.cassandra.index.sai.disk.SSTableIndex;
 import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
@@ -64,14 +65,14 @@ public class IndexViewManager
      *
      * @param oldSSTables A set of SSTables to remove.
      * @param newSSTableContexts A set of SSTableContexts to add to tracker.
-     * @param validate if true, per-column index files' header and footer will 
be validated.
+     * @param validation Controls how indexes should be validated
      *
      * @return A set of SSTables which have attached to them invalid index 
components.
      */
-    public Collection<SSTableContext> update(Collection<SSTableReader> 
oldSSTables, Collection<SSTableContext> newSSTableContexts, boolean validate)
+    public Collection<SSTableContext> update(Collection<SSTableReader> 
oldSSTables, Collection<SSTableContext> newSSTableContexts, IndexValidation 
validation)
     {
         // Valid indexes on the left and invalid SSTable contexts on the 
right...
-        Pair<Collection<SSTableIndex>, Collection<SSTableContext>> indexes = 
context.getBuiltIndexes(newSSTableContexts, validate);
+        Pair<Collection<SSTableIndex>, Collection<SSTableContext>> indexes = 
context.getBuiltIndexes(newSSTableContexts, validation);
 
         View currentView, newView;
         Collection<SSTableIndex> newViewIndexes = new HashSet<>();
@@ -129,7 +130,7 @@ public class IndexViewManager
             index.markObsolete();
         }
 
-        update(toRemove, Collections.emptyList(), false);
+        update(toRemove, Collections.emptyList(), IndexValidation.NONE);
     }
 
     /**
diff --git a/test/unit/org/apache/cassandra/index/sai/SAITester.java 
b/test/unit/org/apache/cassandra/index/sai/SAITester.java
index 903ef1baab..d1616f49f9 100644
--- a/test/unit/org/apache/cassandra/index/sai/SAITester.java
+++ b/test/unit/org/apache/cassandra/index/sai/SAITester.java
@@ -75,6 +75,7 @@ import org.apache.cassandra.index.Index;
 import org.apache.cassandra.index.sai.disk.SSTableIndex;
 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.format.OnDiskFormat;
 import org.apache.cassandra.index.sai.disk.format.Version;
 import org.apache.cassandra.index.sai.disk.v1.V1OnDiskFormat;
 import org.apache.cassandra.index.sai.disk.v1.segment.SegmentBuilder;
@@ -127,13 +128,13 @@ public abstract class SAITester extends CQLTester
                                                                             
.build();
 
     protected static final Injections.Counter perSSTableValidationCounter = 
Injections.newCounter("PerSSTableValidationCounter")
-                                                                               
       .add(newInvokePoint().onClass(IndexDescriptor.class)
-                                                                               
                            .onMethod("validatePerSSTableComponents"))
+                                                                               
       .add(newInvokePoint().onClass(OnDiskFormat.class)
+                                                                               
                            .onMethod("validatePerSSTableIndexComponents"))
                                                                                
       .build();
 
     protected static final Injections.Counter perColumnValidationCounter = 
Injections.newCounter("PerColumnValidationCounter")
-                                                                               
      .add(newInvokePoint().onClass(IndexDescriptor.class)
-                                                                               
                           .onMethod("validatePerIndexComponents"))
+                                                                               
      .add(newInvokePoint().onClass(OnDiskFormat.class)
+                                                                               
                           .onMethod("validatePerColumnIndexComponents"))
                                                                                
      .build();
 
     private static Randomization random;
@@ -320,7 +321,8 @@ public abstract class SAITester extends CQLTester
         for (SSTableReader sstable : cfs.getLiveSSTables())
         {
             IndexDescriptor indexDescriptor = IndexDescriptor.create(sstable);
-            if (!indexDescriptor.validatePerSSTableComponentsChecksum() || 
!indexDescriptor.validatePerIndexComponentsChecksum(indexContext))
+            if 
(!indexDescriptor.validatePerSSTableComponents(IndexValidation.CHECKSUM)
+                || !indexDescriptor.validatePerIndexComponents(indexContext, 
IndexValidation.CHECKSUM))
                 return false;
         }
         return true;
@@ -333,7 +335,8 @@ public abstract class SAITester extends CQLTester
         for (SSTableReader sstable : cfs.getLiveSSTables())
         {
             IndexDescriptor indexDescriptor = IndexDescriptor.create(sstable);
-            if (!indexDescriptor.validatePerSSTableComponents() || 
!indexDescriptor.validatePerIndexComponents(indexContext))
+            if 
(!indexDescriptor.validatePerSSTableComponents(IndexValidation.HEADER_FOOTER)
+                || !indexDescriptor.validatePerIndexComponents(indexContext, 
IndexValidation.HEADER_FOOTER))
                 return false;
         }
         return true;
diff --git 
a/test/unit/org/apache/cassandra/index/sai/disk/v1/trie/TrieValidationTest.java 
b/test/unit/org/apache/cassandra/index/sai/disk/v1/trie/TrieValidationTest.java
new file mode 100644
index 0000000000..f7378077a9
--- /dev/null
+++ 
b/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
+{
+    private 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 static 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 static ByteSource createMultiPart(ByteComparable.Version version, 
String... parts)
+    {
+        ByteSource [] byteSources = new ByteSource[parts.length];
+        for (int index = 0; index < parts.length; index++)
+            byteSources[index] = 
UTF8Type.instance.asComparableBytes(UTF8Type.instance.fromString(parts[index]), 
version);
+        return ByteSource.withTerminator(ByteSource.TERMINATOR, byteSources);
+    }
+
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


Reply via email to