Skip to content

CNDB-13534: Add RequestFailureReason.FEATURE_NEEDS_INDEX_REBUILD #1781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 10, 2025

Conversation

driftx
Copy link

@driftx driftx commented May 29, 2025

The main difference was porting FeatureNeedsIndexRebuildException.

@djatnieks
Copy link

I looked at the test failures reported above by Butler and they all look like known failing or flaky tests on the 5.0 test failure list

I don't think Butler knows how to do a correct comparison with main-5.0 - meaning I think it always compares PR results to main.

One thing, if you didn't do it already, is to run the "verify" script that I think I mentioned previously - it identified all the unit and distributed tests that were touched by the most recent commit and executes them. For a relatively large commit like this, that's my typical method of verifying the port

@djatnieks djatnieks requested review from djatnieks and removed request for djatnieks May 31, 2025 00:06
@djatnieks
Copy link

I checked out the branch and got a compile error:

    [javac] /Users/danj/projects/cc-main-5.0/test/unit/org/apache/cassandra/index/sai/disk/v1/kdtree/KDTreeIndexBuilder.java:194: error: cannot find symbol
    [javac]             assertThat(searcher, is(instanceOf(KDTreeIndexSearcher.class)));
    [javac]                                     ^
    [javac]   symbol:   method instanceOf(Class<KDTreeIndexSearcher>)
    [javac]   location: class KDTreeIndexBuilder

Then I realized the CI build has the same error - weird that it ran tests if the build part had a problem 🤷🏻

@driftx
Copy link
Author

driftx commented Jun 2, 2025

Then I realized the CI build has the same error - weird that it ran tests if the build part had a problem 🤷🏻

I think that was because your commits got in and caused a small conflict that I resolved in the web interface. I've rebased and pushed it now.

Copy link

@djatnieks djatnieks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SAI_CURRENT_VERSION property in CassandraRelevantProperties should be placed in alphabetical order in 5.0.

The new request failure reason should be used when the on-disk format of an
index in a replica doesn't support a requested feature.

Also, rename org.apache.cassandra.index.sai.disk.format.Version.latest() to Version.current(),
and add separate Version.CURRENT and Version.LATEST constants.
@djatnieks
Copy link

Butler results are easier to read now - there are some failures in these new tests that were added FeaturesVersionSupport*Test - maybe something in 5.0 that is making those fail?
e.g.
http://butler-stargazer.aws.dsinternal.org/#/ci/upstream/workflow/ds-cassandra-pr-gate/job/CNDB-14282/failure/junit.framework.TestSuite?test_name=org.apache.cassandra.distributed.test.sai.features.FeaturesVersionSupportAATest-_jdk11

@driftx
Copy link
Author

driftx commented Jun 10, 2025

That wasn't needed in main because there's no BM25 feature to test, and so no backwards compatibility issues to handle.

Copy link

@cassci-bot
Copy link

❌ Build ds-cassandra-pr-gate/PR-1781 rejected by Butler


15 new test failure(s) in 5 builds
See build details here


Found 15 new test failures

Test Explanation Branch history Upstream history
....r.PendingAntiCompactionTest.testRetriesTimeout regression 🔴🔴🔴🔵
...adCommitLogAndSSTablesWithDroppedColumnTestCC40 regression 🔴🔴🔴🔴
...adCommitLogAndSSTablesWithDroppedColumnTestCC50 regression 🔴🔴🔴🔴
...oadCommitLogAndSSTablesWithDroppedColumnTestDSE regression 🔴🔴🔴🔴
...thRestartTest.testReadingValuesOfDroppedColumns regression 🔴🔴🔴🔴
o.a.c.i.s.c.BM25Test.testCollections regression 🔴🔴🔴🔴
...est.testOrderingSeveralSSTablesWithMapPredicate regression 🔴🔴🔴🔴
...uldTolerateDuplicatedRowIDsAfterMemtableUpdates regression 🔴🔴🔴🔴
...ectorTypeTest.newJVectorOptionsTestVersion2[ca] regression 🔴🔴🔴🔴
...ectorTypeTest.newJVectorOptionsTestVersion2[dc] regression 🔴🔴🔴🔴
...ectorTypeTest.newJVectorOptionsTestVersion4[ca] regression 🔴🔴🔴🔴
...ectorTypeTest.newJVectorOptionsTestVersion4[dc] regression 🔴🔴🔴🔴
...lushingTest.testFlushingLargeStaleMemtableIndex regression 🔴🔴🔴🔴
...cySSTableTest.testVerifyOldDroppedTupleSSTables regression 🔴🔴🔴🔴
o.a.c.t.SSTablePartitionsTest.testDirectory regression 🔴🔴🔴🔴

Found 115 known test failures

@driftx driftx merged commit dce2e0c into main-5.0 Jun 10, 2025
572 of 586 checks passed
@driftx driftx deleted the CNDB-14282 branch June 10, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants