You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "dcapwell (via GitHub)" <gi...@apache.org> on 2025/04/18 22:00:05 UTC
[PR] CASSANDRA-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
dcapwell opened a new pull request, #4105:
URL: https://github.com/apache/cassandra/pull/4105
(no comment)
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#issuecomment-2821904382
i am going to move the fuzz test logic out of this patch, as it keeps being flakey due to ZCS allocating direct memory and failing. I am testing removing this allocation (which should make the test no longer flakey), but that has nothing to do with SAI so best to do as another ticket.
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051183219
##########
test/unit/org/apache/cassandra/utils/CassandraGenerators.java:
##########
@@ -563,11 +565,39 @@ public static class CompactionParamsBuilder
Map<String, String> options = new HashMap<>();
if (nextBoolean(rnd))
options.putAll(sizeTieredOptions.generate(rnd));
+ int maxSSTableSizeInMB = LeveledCompactionStrategy.DEFAULT_MAX_SSTABLE_SIZE_MIB;
if (nextBoolean(rnd))
+ {
// size in mb
- options.put(LeveledCompactionStrategy.SSTABLE_SIZE_OPTION, SourceDSL.integers().between(1, 2_000).generate(rnd).toString());
+ maxSSTableSizeInMB = SourceDSL.integers().between(1, 2_000).generate(rnd);
+ options.put(LeveledCompactionStrategy.SSTABLE_SIZE_OPTION, Integer.toString(maxSSTableSizeInMB));
+ }
if (nextBoolean(rnd))
- options.put(LeveledCompactionStrategy.LEVEL_FANOUT_SIZE_OPTION, SourceDSL.integers().between(1, 100).generate(rnd).toString());
+ {
+ // there is a relationship between sstable size and fanout, so respect it
Review Comment:
one seed randomly hit this case, so fix it so we don't happen again...
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051182748
##########
test/unit/org/apache/cassandra/repair/FuzzTestBase.java:
##########
@@ -557,53 +554,12 @@ static RepairOption previewOption(RandomSource rs, Cluster.Node coordinator, Str
private static RepairOption repairOption(RandomSource rs, Cluster.Node coordinator, String ks, Gen<List<String>> tablesGen, Gen<RepairType> repairTypeGen, Gen<PreviewType> previewTypeGen, Gen<RepairParallelism> repairParallelismGen)
{
- RepairType type = repairTypeGen.next(rs);
Review Comment:
rather than having different tests randomize repair stuff, i pulled this out into `RepairGenerators` so this logic could be reused
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051172285
##########
test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java:
##########
@@ -371,6 +371,8 @@ public void test() throws IOException
.add(this::selectTokenRange))
.addIf(State::hasEnoughMemtable, StatefulASTBase::flushTable)
.addIf(State::hasEnoughSSTables, StatefulASTBase::compactTable)
+ .addAllIf(BaseState::allowRepair, b -> b.add(StatefulASTBase::incrementalRepair)
+ .addIf(BaseState::isConsistent, StatefulASTBase::previewRepair))
Review Comment:
preview repairs fails if it finds a mismatch, so we can only call it safely in the test when we *know* we should be consistent.
for accord, we flush during the `prepare` phase, which is *before* we do any barriers, so we can't just say "accord makes us consistent", because we might not be (you need to go through accord to be consistent).
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell closed pull request #4105: CASSANDRA-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair
URL: https://github.com/apache/cassandra/pull/4105
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051182542
##########
test/distributed/org/apache/cassandra/fuzz/sai/AccordFullMultiNodeSAITest.java:
##########
@@ -25,7 +25,7 @@
import org.apache.cassandra.harry.gen.SchemaGenerators;
import org.apache.cassandra.service.consensus.TransactionalMode;
-@Ignore("CASSANDRA-20567: Repair is failing due to missing SAI index files when using zero copy streaming")
+@Ignore("It was believed that these tests were failing due to CASSANDRA-20567, but in fixing that issue it was found that the tests are still failing! Harry is detecting an incorrect response...")
Review Comment:
was really hoping fixing this patch would fix these tests, so we have to root cause more.
I had a ton of seeds from CI, and non reproduced locally =(
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051170031
##########
src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java:
##########
@@ -218,7 +218,7 @@ private void flushVectorIndex(long startTime, Stopwatch stopwatch) throws IOExce
private void completeIndexFlush(long cellCount, long startTime, Stopwatch stopwatch) throws IOException
{
// create a completion marker indicating that the index is complete and not-empty
- ColumnCompletionMarkerUtil.create(indexDescriptor, indexIdentifier, false);
+ ColumnCompletionMarkerUtil.create(indexDescriptor, indexIdentifier, cellCount == 0);
Review Comment:
this is the actual patch for the bug fix. Repair is causing multiple sstables to be generated, and the tests do partial partition/row updates, so SAI sees a column as having data, but it didn't write anything for *this range*... this leads to `cellCount == 0`
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#discussion_r2051172529
##########
test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java:
##########
@@ -144,7 +149,7 @@ protected static String nextKeyspace()
protected void clusterConfig(IInstanceConfig config)
{
-
+ config.set("repair.retries.max_attempts", Integer.MAX_VALUE);
Review Comment:
these are jvm-dtest clusters, retry non stop until it works... these tests don't do chaos testing, so its really the "happy path" case
--
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-20567: SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on PR #4105:
URL: https://github.com/apache/cassandra/pull/4105#issuecomment-2828873103
merged
--
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