You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "sunil9977 (via GitHub)" <gi...@apache.org> on 2025/10/14 17:40:52 UTC
[PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
sunil9977 opened a new pull request, #4426:
URL: https://github.com/apache/cassandra/pull/4426
CASSANDRA-20949
```
Adds verification of SAI components to the nodetool verify command. Skips system keyspaces and tables without SAI indexes.
patch by Sunil Ramchandra Pawar; reviewed by Caleb Rackliffe for CASSANDRA-20949
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2613974056
##########
test/unit/org/apache/cassandra/tools/nodetool/VerifyTest.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.tools.nodetool;
+
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import com.google.common.collect.Iterables;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.index.sai.StorageAttachedIndex;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
+import org.apache.cassandra.io.sstable.Component;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.io.util.File;
+
+import static org.apache.cassandra.tools.ToolRunner.invokeNodetool;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class VerifyTest extends CQLTester
+{
+
+ @BeforeClass
+ public static void setup() throws Throwable
+ {
+ requireNetwork();
+ startJMXServer();
+ }
+
+ @Test
+ public void testVerifyDefault()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY(pk,ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+
+ flush(keyspace());
+
+ invokeNodetool("verify", "--force", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnly()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode: verify only SAI components
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyIncludeSai()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // Include-sai mode: verify both data files and SAI components
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnlyWithoutSaiIndex()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ // No SAI index created
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode on table without SAI should succeed (skipped)
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyConflictingFlags()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+
+ // Both --sai-only and --include-sai should fail
+ invokeNodetool("verify", "--force", "--sai-only", "--include-sai", keyspace(), currentTable())
+ .asserts().failure();
+ }
+
+ @Test
+ public void testVerifySaiWithCorruption ()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
+ StorageAttachedIndexGroup group = StorageAttachedIndexGroup.getIndexGroup(cfs);
+ assertNotNull("SAI group should exist", group);
+
+ SSTableReader sstable = Iterables.getOnlyElement(cfs.getLiveSSTables());
+ Set<Component> saiComponents = StorageAttachedIndexGroup.getLiveComponents(sstable,
+ group.getIndexes().stream().map(i -> (StorageAttachedIndex)i).collect(Collectors.toSet()));
+
+ assertFalse("SAI components should exist", saiComponents.isEmpty());
+
+ Component componentToCorrupt = saiComponents.iterator().next();
+ File saiFile = sstable.descriptor.fileFor(componentToCorrupt);
+
+
+ assertTrue("SAI file should exist", saiFile.exists());
+
+
+ try (FileChannel channel = saiFile.newReadWriteChannel())
+ {
+ channel.truncate(10);
+ }
+ catch (IOException e)
+ {
+ throw new RuntimeException(e);
+ }
+
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).asserts().failure();
+
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).asserts().failure();
Review Comment:
Thanks. Replaced with the above.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608466504
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -1493,18 +1504,39 @@ 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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
+
+ // If this table shouldn’t be verified, we skip early
+ if (options.onlySai && skipSaiCheck)
+ {
+ logger.info("Skipping SAI validation for table {} (system keyspace or no SAI index).", 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))
Review Comment:
```suggestion
try (IVerifier verifier = sstable.getVerifier(cfs, new OutputHandler.LogOutput(), false, options))
```
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -1493,18 +1504,39 @@ 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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
+
+ // If this table shouldn’t be verified, we skip early
+ if (options.onlySai && skipSaiCheck)
+ {
+ logger.info("Skipping SAI validation for table {} (system keyspace or no SAI index).", 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);
}
+
Review Comment:
nit: extra newline
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611738336
##########
src/java/org/apache/cassandra/service/StorageServiceMBean.java:
##########
@@ -452,7 +452,7 @@ 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;
- 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;
Review Comment:
yeah, thats what we do with repair; we create a new one and have the old ones just call the new one with the defaults.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608464476
##########
test/unit/org/apache/cassandra/tools/nodetool/VerifyTest.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.tools.nodetool;
+
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import com.google.common.collect.Iterables;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.index.sai.StorageAttachedIndex;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
+import org.apache.cassandra.io.sstable.Component;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.io.util.File;
+
+import static org.apache.cassandra.tools.ToolRunner.invokeNodetool;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class VerifyTest extends CQLTester
+{
+
+ @BeforeClass
+ public static void setup() throws Throwable
+ {
+ requireNetwork();
+ startJMXServer();
+ }
+
+ @Test
+ public void testVerifyDefault()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY(pk,ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+
+ flush(keyspace());
+
+ invokeNodetool("verify", "--force", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnly()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode: verify only SAI components
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyIncludeSai()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // Include-sai mode: verify both data files and SAI components
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnlyWithoutSaiIndex()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ // No SAI index created
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode on table without SAI should succeed (skipped)
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyConflictingFlags()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+
+ // Both --sai-only and --include-sai should fail
+ invokeNodetool("verify", "--force", "--sai-only", "--include-sai", keyspace(), currentTable())
+ .asserts().failure();
Review Comment:
Could also do...
```
ToolRunner.ToolResult result = invokeNodetool("verify", "--force", "--sai-only", "--include-sai", keyspace(), currentTable());
result.asserts().failure();
result.getStdout().contains("Cannot specify both --sai-only and --include-sai");
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611330284
##########
src/java/org/apache/cassandra/tools/nodetool/Verify.java:
##########
@@ -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");
Review Comment:
Setting `--sai-only` could implicitly set `--include-sai` I guess, but still might be good to reject at this level, since setting both might have been unintentional and we want to point that out to the user?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2613987524
##########
src/java/org/apache/cassandra/service/StorageServiceMBean.java:
##########
@@ -452,7 +452,7 @@ 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;
- 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;
Review Comment:
Kept the original Verify signature along with the new one for backward compatibility.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2613979315
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -669,6 +671,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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
Review Comment:
Excluded the check for system keyspaces.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611333889
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -669,6 +671,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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
Review Comment:
Yeah, wouldn't hurt ATM, but don't think there's a good reason to exclude system keyspaces...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608564239
##########
src/java/org/apache/cassandra/service/StorageServiceMBean.java:
##########
@@ -452,7 +452,7 @@ 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;
- 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;
Review Comment:
this is a breaking change, won't this impact sidecar?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608561211
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -1493,18 +1504,39 @@ 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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
Review Comment:
same question as above, if there is a index why exclude system keyspaces?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608473435
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -669,6 +671,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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
Review Comment:
why do we skip system tables that have SAI indexes? We don't have any atm, just thinking about future
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2613980909
##########
test/unit/org/apache/cassandra/tools/nodetool/VerifyTest.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.tools.nodetool;
+
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import com.google.common.collect.Iterables;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.index.sai.StorageAttachedIndex;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
+import org.apache.cassandra.io.sstable.Component;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.io.util.File;
+
+import static org.apache.cassandra.tools.ToolRunner.invokeNodetool;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class VerifyTest extends CQLTester
+{
+
+ @BeforeClass
+ public static void setup() throws Throwable
+ {
+ requireNetwork();
+ startJMXServer();
+ }
+
+ @Test
+ public void testVerifyDefault()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY(pk,ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+
+ flush(keyspace());
+
+ invokeNodetool("verify", "--force", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnly()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode: verify only SAI components
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyIncludeSai()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // Include-sai mode: verify both data files and SAI components
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnlyWithoutSaiIndex()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ // No SAI index created
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode on table without SAI should succeed (skipped)
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyConflictingFlags()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+
+ // Both --sai-only and --include-sai should fail
+ invokeNodetool("verify", "--force", "--sai-only", "--include-sai", keyspace(), currentTable())
+ .asserts().failure();
Review Comment:
Thanks. Replaced with above.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "sunil9977 (via GitHub)" <gi...@apache.org>.
sunil9977 commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2613982779
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -1493,18 +1504,39 @@ 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 || SchemaConstants.isSystemKeyspace(cfs.getKeyspaceName());
Review Comment:
Thanks. Addressed 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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#issuecomment-3647924374
Committed as https://github.com/apache/cassandra/commit/a84abe7ba13fead1d889ceb2347357f971719e98
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611742801
##########
src/java/org/apache/cassandra/tools/nodetool/Verify.java:
##########
@@ -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");
Review Comment:
im ok if we want to keep this, but similar APIs have added flags to change the defaults; maybe we *should* validate SAI by default? If we make that a flag than `onlySai` becomes unsafe
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608568335
##########
src/java/org/apache/cassandra/tools/nodetool/Verify.java:
##########
@@ -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");
Review Comment:
why is this an issue? We do `options.onlySai || options.includeSai`
I would think `--sai-only` would implicitly set `includeSai = true`
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611742801
##########
src/java/org/apache/cassandra/tools/nodetool/Verify.java:
##########
@@ -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");
Review Comment:
im ok if we want to keep this, but similar APIs have added flags to change the defaults (over time as they get used); maybe we *should* validate SAI by default? If we make that a flag than `onlySai` becomes unsafe
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz closed pull request #4426: CASSANDRA-20949: Extend nodetool verify to validate SAI files
URL: https://github.com/apache/cassandra/pull/4426
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2608456814
##########
test/unit/org/apache/cassandra/tools/nodetool/VerifyTest.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.tools.nodetool;
+
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import com.google.common.collect.Iterables;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.index.sai.StorageAttachedIndex;
+import org.apache.cassandra.index.sai.StorageAttachedIndexGroup;
+import org.apache.cassandra.io.sstable.Component;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.io.util.File;
+
+import static org.apache.cassandra.tools.ToolRunner.invokeNodetool;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class VerifyTest extends CQLTester
+{
+
+ @BeforeClass
+ public static void setup() throws Throwable
+ {
+ requireNetwork();
+ startJMXServer();
+ }
+
+ @Test
+ public void testVerifyDefault()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY(pk,ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+
+ flush(keyspace());
+
+ invokeNodetool("verify", "--force", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnly()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode: verify only SAI components
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyIncludeSai()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // Include-sai mode: verify both data files and SAI components
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifySaiOnlyWithoutSaiIndex()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ // No SAI index created
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ // SAI-only mode on table without SAI should succeed (skipped)
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).assertOnCleanExit();
+ }
+
+ @Test
+ public void testVerifyConflictingFlags()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+
+ // Both --sai-only and --include-sai should fail
+ invokeNodetool("verify", "--force", "--sai-only", "--include-sai", keyspace(), currentTable())
+ .asserts().failure();
+ }
+
+ @Test
+ public void testVerifySaiWithCorruption ()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, a int, b int, PRIMARY KEY (pk, ck))");
+ createIndex("CREATE INDEX idx1 ON %s(a) USING 'sai'");
+
+ for (int i = 0; i < 10; i++)
+ {
+ execute("INSERT INTO %s (pk, ck, a, b) VALUES (?, ?, ?, ?)", i, i * 2, i * 3, i * 4);
+ }
+ flush(keyspace());
+
+ ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
+ StorageAttachedIndexGroup group = StorageAttachedIndexGroup.getIndexGroup(cfs);
+ assertNotNull("SAI group should exist", group);
+
+ SSTableReader sstable = Iterables.getOnlyElement(cfs.getLiveSSTables());
+ Set<Component> saiComponents = StorageAttachedIndexGroup.getLiveComponents(sstable,
+ group.getIndexes().stream().map(i -> (StorageAttachedIndex)i).collect(Collectors.toSet()));
+
+ assertFalse("SAI components should exist", saiComponents.isEmpty());
+
+ Component componentToCorrupt = saiComponents.iterator().next();
+ File saiFile = sstable.descriptor.fileFor(componentToCorrupt);
+
+
+ assertTrue("SAI file should exist", saiFile.exists());
+
+
+ try (FileChannel channel = saiFile.newReadWriteChannel())
+ {
+ channel.truncate(10);
+ }
+ catch (IOException e)
+ {
+ throw new RuntimeException(e);
+ }
+
+ invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).asserts().failure();
+
+ invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).asserts().failure();
Review Comment:
I'd replace the above w/ the following, just to make sure we get the right message and have a successful run without checking SAI:
```
invokeNodetool("verify", "--force", keyspace(), currentTable()).asserts().success();
invokeNodetool("verify", "--force", "--sai-only", keyspace(), currentTable()).asserts().failure().errorContains("file truncated");
invokeNodetool("verify", "--force", "--include-sai", keyspace(), currentTable()).asserts().failure().errorContains("file truncated");
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20949: Extend nodetool verify to validate SAI files [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4426:
URL: https://github.com/apache/cassandra/pull/4426#discussion_r2611344742
##########
src/java/org/apache/cassandra/service/StorageServiceMBean.java:
##########
@@ -452,7 +452,7 @@ 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;
- 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;
Review Comment:
That's fair...might be better to just keep the existing signature and add a new one. Method with the existing signature in `StorageService` can just default them to `false`.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org