Extend nodetool verify to (optionally) validate SAI files

patch by Sunil Ramchandra Pawar; reviewed by Caleb Rackliffe and David Capwell for CASSANDRA-20949

Author: sunil9977

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Extend nodetool verify to (optionally) validate SAI files (CASSANDRA-20949)
  * Fix CompressionDictionary being closed while still in use (CASSANDRA-21047)
  * When updating a multi cell collection element, if the update is rejected then the shared Row.Builder is not freed causing all future mutations to be rejected (CASSANDRA-21055)
  * Schema annotations escape validation on CREATE and ALTER DDL statements (CASSANDRA-21046)

src/java/org/apache/cassandra/db/compaction/CompactionManager.java

@@ -89,6 +89,7 @@
 import org.apache.cassandra.dht.Token;
 import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.index.SecondaryIndexBuilder;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
 import org.apache.cassandra.io.sstable.Descriptor;
 import org.apache.cassandra.io.sstable.ISSTableScanner;
 import org.apache.cassandra.io.sstable.IScrubber;
@@ -669,6 +670,15 @@ public void execute(LifecycleTransaction input)
     public AllSSTableOpStatus performVerify(ColumnFamilyStore cfs, IVerifier.Options options) throws InterruptedException, ExecutionException
     {
         assert !cfs.isIndex();
+        StorageAttachedIndexGroup indexGroup = StorageAttachedIndexGroup.getIndexGroup(cfs);
+        boolean skipSaiCheck = indexGroup == null;
+        if (options.onlySai && skipSaiCheck)
+        {
+            logger.info("Skipping table {} during SAI-only verify because it has no SAI indexes.", cfs.getTableName());
+            return AllSSTableOpStatus.SUCCESSFUL;
+
+        }
+
         return parallelAllSSTableOperation(cfs, new OneSSTableOperation()
         {
             @Override
@@ -1493,17 +1503,37 @@ void scrubOne(ColumnFamilyStore cfs, LifecycleTransaction modifier, IScrubber.Op
     @VisibleForTesting
     void verifyOne(ColumnFamilyStore cfs, SSTableReader sstable, IVerifier.Options options, ActiveCompactionsTracker activeCompactions)
     {
+
+        StorageAttachedIndexGroup indexGroup = StorageAttachedIndexGroup.getIndexGroup(cfs);
+        boolean skipSaiCheck = indexGroup == null;
+
+        // Skip early if no SAI indexes on table
+        if (options.onlySai && skipSaiCheck)
+        {
+            logger.info("Skipping SAI validation for table {} because it has no SAI indexes.", cfs.getTableName());
+            return;
+        }
+
         CompactionInfo.Holder verifyInfo = null;
-        try (IVerifier verifier = sstable.getVerifier(cfs, new OutputHandler.LogOutput(), false, options))
+
+        if (!options.onlySai)
         {
-            verifyInfo = verifier.getVerifyInfo();
-            activeCompactions.beginCompaction(verifyInfo);
-            verifier.verify();
+            try (IVerifier verifier = sstable.getVerifier(cfs, new OutputHandler.LogOutput(), false, options))
+            {
+                verifyInfo = verifier.getVerifyInfo();
+                activeCompactions.beginCompaction(verifyInfo);
+                verifier.verify();
+            }
+            finally
+            {
+                if (verifyInfo != null)
+                    activeCompactions.finishCompaction(verifyInfo);
+            }
         }
-        finally
+
+        if ((options.onlySai || options.includeSai) && !skipSaiCheck)
         {
-            if (verifyInfo != null)
-                activeCompactions.finishCompaction(verifyInfo);
+            cfs.indexManager.validateSSTableAttachedIndexes(Collections.singleton(sstable), true, true);
         }
     }
 

src/java/org/apache/cassandra/io/sstable/IVerifier.java

@@ -60,6 +60,16 @@ class Options
          */
         public final boolean quick;
 
+        /**
+         * To verify only SAI checksum
+         */
+        public final boolean onlySai;
+
+        /**
+         * To include SAI verification along with data files
+         */
+        public final boolean includeSai;
+
         public final Function<String, ? extends Collection<Range<Token>>> tokenLookup;
 
         private Options(boolean invokeDiskFailurePolicy,
@@ -68,14 +78,21 @@ private Options(boolean invokeDiskFailurePolicy,
                         boolean mutateRepairStatus,
                         boolean checkOwnsTokens,
                         boolean quick,
+                        boolean onlySai,
+                        boolean includeSai,
                         Function<String, ? extends Collection<Range<Token>>> tokenLookup)
         {
+            if (onlySai && includeSai)
+                throw new IllegalArgumentException("onlySai and includeSai both cannot be true at a time.");
+
             this.invokeDiskFailurePolicy = invokeDiskFailurePolicy;
             this.extendedVerification = extendedVerification;
             this.checkVersion = checkVersion;
             this.mutateRepairStatus = mutateRepairStatus;
             this.checkOwnsTokens = checkOwnsTokens;
             this.quick = quick;
+            this.onlySai = onlySai;
+            this.includeSai = includeSai;
             this.tokenLookup = tokenLookup;
         }
 
@@ -89,6 +106,8 @@ public String toString()
                    ", mutateRepairStatus=" + mutateRepairStatus +
                    ", checkOwnsTokens=" + checkOwnsTokens +
                    ", quick=" + quick +
+                   ", onlySai=" + onlySai +
+                   ", includeSai=" + includeSai +
                    '}';
         }
 
@@ -100,6 +119,8 @@ public static class Builder
             private boolean mutateRepairStatus = false; // mutating repair status can be dangerous
             private boolean checkOwnsTokens = false;
             private boolean quick = false;
+            private boolean onlySai = false;
+            private boolean includeSai = false;
             private Function<String, ? extends Collection<Range<Token>>> tokenLookup = StorageService.instance::getLocalAndPendingRanges;
 
             public Builder invokeDiskFailurePolicy(boolean param)
@@ -138,6 +159,18 @@ public Builder quick(boolean param)
                 return this;
             }
 
+            public Builder onlySai(boolean param)
+            {
+                this.onlySai = param;
+                return this;
+            }
+
+            public Builder includeSai(boolean param)
+            {
+                this.includeSai = param;
+                return this;
+            }
+
             public Builder tokenLookup(Function<String, ? extends Collection<Range<Token>>> tokenLookup)
             {
                 this.tokenLookup = tokenLookup;
@@ -146,7 +179,7 @@ public Builder tokenLookup(Function<String, ? extends Collection<Range<Token>>>
 
             public Options build()
             {
-                return new Options(invokeDiskFailurePolicy, extendedVerification, checkVersion, mutateRepairStatus, checkOwnsTokens, quick, tokenLookup);
+                return new Options(invokeDiskFailurePolicy, extendedVerification, checkVersion, mutateRepairStatus, checkOwnsTokens, quick, onlySai, includeSai, tokenLookup);
             }
         }
     }

src/java/org/apache/cassandra/service/StorageService.java

@@ -2690,18 +2690,28 @@ public int scrub(boolean disableSnapshot, IScrubber.Options options, int jobs, S
     @Deprecated(since = "4.0")
     public int verify(boolean extendedVerify, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
     {
-        return verify(extendedVerify, false, false, false, false, false, keyspaceName, tableNames);
+        return verify(extendedVerify, false, false, false, false, false, false, false, keyspaceName, tableNames);
     }
 
+    /**
+     * Kept for backward compatibility with existing clients.
+     */
     public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
+    {
+        return verify(extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, false, false, keyspaceName, tableNames);
+    }
+
+    public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, boolean onlySai, boolean includeSai, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
     {
         CompactionManager.AllSSTableOpStatus status = CompactionManager.AllSSTableOpStatus.SUCCESSFUL;
         IVerifier.Options options = IVerifier.options().invokeDiskFailurePolicy(diskFailurePolicy)
                                              .extendedVerification(extendedVerify)
                                              .checkVersion(checkVersion)
                                              .mutateRepairStatus(mutateRepairStatus)
                                              .checkOwnsTokens(checkOwnsTokens)
-                                             .quick(quick).build();
+                                             .quick(quick)
+                                             .onlySai(onlySai)
+                                             .includeSai(includeSai).build();
         logger.info("Staring {} on {}.{} with options = {}", OperationType.VERIFY, keyspaceName, Arrays.toString(tableNames), options);
         for (ColumnFamilyStore cfStore : getValidColumnFamilies(false, false, keyspaceName, tableNames))
         {

src/java/org/apache/cassandra/service/StorageServiceMBean.java

@@ -452,8 +452,17 @@ default int scrub(boolean disableSnapshot, boolean skipCorrupted, boolean checkD
      * The entire sstable will be read to ensure each cell validates if extendedVerify is true
      */
     public int verify(boolean extendedVerify, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException;
+
+    /**
+     * Kept for backward compatibility with existing clients.
+     */
     public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException;
 
+    /**
+     * Verify checksums of the given keyspace with extended options including SAI index validation.
+     */
+    public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, boolean onlySai, boolean includeSai, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException;
+
     /**
      * Rewrite all sstables to the latest version.
      * Unlike scrub, it doesn't skip bad rows and do not snapshot sstables first.

src/java/org/apache/cassandra/tools/NodeProbe.java

@@ -391,9 +391,9 @@ public int scrub(boolean disableSnapshot, boolean skipCorrupted, boolean checkDa
         return ssProxy.scrub(disableSnapshot, skipCorrupted, checkData, reinsertOverflowedTTL, jobs, keyspaceName, tables);
     }
 
-    public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
+    public int verify(boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, boolean onlySai, boolean includeSai, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
     {
-        return ssProxy.verify(extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, keyspaceName, tableNames);
+        return ssProxy.verify(extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, onlySai, includeSai, keyspaceName, tableNames);
     }
 
     public int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion, long maxSSTableTimestamp, int jobs, String... tableNames) throws IOException, ExecutionException, InterruptedException
@@ -434,10 +434,10 @@ public void scrub(PrintStream out, boolean disableSnapshot, boolean skipCorrupte
                 "scrubbing");
     }
 
-    public void verify(PrintStream out, boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
+    public void verify(PrintStream out, boolean extendedVerify, boolean checkVersion, boolean diskFailurePolicy, boolean mutateRepairStatus, boolean checkOwnsTokens, boolean quick, boolean onlySai, boolean includeSai, String keyspaceName, String... tableNames) throws IOException, ExecutionException, InterruptedException
     {
         perform(out, keyspaceName,
-                () -> verify(extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, keyspaceName, tableNames),
+                () -> verify(extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, onlySai, includeSai, keyspaceName, tableNames),
                 "verifying");
     }
 

src/java/org/apache/cassandra/tools/nodetool/Verify.java

@@ -78,6 +78,16 @@ public class Verify extends AbstractCommand
             description = "Do a quick check - avoid reading all data to verify checksums")
     private boolean quick = false;
 
+    @Option(paramLabel = "sai_only",
+            names = { "-s", "--sai-only"},
+            description = "Verify only sai index")
+    private boolean onlySai = false;
+
+    @Option(paramLabel = "include_sai",
+            names = { "-i", "--include-sai"},
+            description = "Include SAI index verification along with data files")
+    private boolean includeSai = false;
+
     @Override
     public void execute(NodeProbe probe)
     {
@@ -89,6 +99,9 @@ public void execute(NodeProbe probe)
             System.exit(1);
         }
 
+        if (onlySai && includeSai)
+            throw new IllegalArgumentException("Cannot specify both --sai-only and --include-sai");
+
         List<String> keyspaces = parseOptionalKeyspace(args, probe);
         String[] tableNames = parseOptionalTables(args);
 
@@ -103,7 +116,7 @@ public void execute(NodeProbe probe)
         {
             try
             {
-                probe.verify(out, extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, keyspace, tableNames);
+                probe.verify(out, extendedVerify, checkVersion, diskFailurePolicy, mutateRepairStatus, checkOwnsTokens, quick, onlySai, includeSai, keyspace, tableNames);
             } catch (Exception e)
             {
                 throw new RuntimeException("Error occurred during verifying", e);

test/resources/nodetool/help/verify

@@ -7,8 +7,9 @@ SYNOPSIS
                 [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]
                 [(-u <username> | --username <username>)] verify
                 [(-c | --check-version)] [(-d | --dfp)] [(-e | --extended-verify)]
-                [(-f | --force)] [(-q | --quick)] [(-r | --rsc)]
-                [(-t | --check-tokens)] [--] [<keyspace> <tables>...]
+                [(-f | --force)] [(-i | --include-sai)] [(-q | --quick)] [(-r | --rsc)]
+                [(-s | --sai-only)] [(-t | --check-tokens)] [--] [<keyspace>
+                <tables>...]
 
 OPTIONS
         -c, --check-version
@@ -26,6 +27,9 @@ OPTIONS
         -h <host>, --host <host>
             Node hostname or ip address
 
+        -i, --include-sai
+            Include SAI index verification along with data files
+
         -p <port>, --port <port>
             Remote jmx agent port number
 
@@ -44,6 +48,9 @@ OPTIONS
         -r, --rsc
             Mutate the repair status on corrupt sstables
 
+        -s, --sai-only
+            Verify only sai index
+
         -t, --check-tokens
             Verify that all tokens in sstables are owned by this node