Skip to content

CNDB-16886: Reject BM25 queries in validation when the coordinator is…#2459

Merged
adelapena merged 1 commit into
mainfrom
CNDB-16886-main
Jun 18, 2026
Merged

CNDB-16886: Reject BM25 queries in validation when the coordinator is…#2459
adelapena merged 1 commit into
mainfrom
CNDB-16886-main

Conversation

@adelapena

Copy link
Copy Markdown

What is the issue

Writers reject BM25 queries when the index version if before Version.BM25_EARLIEST, or when the index doesn't have the needed components. They do that by sending RequestFailureReason.FEATURE_NEEDS_INDEX_UPGRADE. If the query succeeds in the writers, there are still more version checks in the coordinator, so everything should be in the right version.

While that behaviour is correct and manages index version issues in a controlled manner, we could also check the current index version at the query validation step done by the coordinator, before actually executing the query. That would allow us to throw a better error message than the obscure ReadFailureException with named error codes, and save us some round trips, in case the coordinator is in the wrong version. The ReadFailureException with named error codes would still be used for cases when only some of the writers or the sstables is in the wrong version.

What does this PR fix and why was it fixed

Reject BM25 queries on Index.validate(ReadCommand) if the current index version doesn't support them. That way they would be rejected on the coordinator before execution, which is faster and produces a descriptive error message, rather than the generic per-replica error codes. The replicas still will do their own validation if the query gets past the coordinator.

@adelapena adelapena self-assigned this Jun 12, 2026
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

Checklist before you submit for review

  • This PR adheres to the Definition of Done
  • Make sure there is a PR and ticket in the CNDB project updating the Converged Cassandra version
  • Use NoSpamLogger for log lines that may appear frequently in the logs
  • Verify test results on Butler
  • Test coverage for new/modified code is > 80%
  • Proper code formatting
  • Proper title for each commit staring with the project-issue number, like CNDB-1234
  • Each commit has a meaningful description
  • Each commit is not very long and contains related changes
  • Renames, moves and reformatting are in distinct commits
  • All new files should contain the IBM copyright header instead of the Apache License one (no DataStax copyright any longer)

@plpesvc-ds

Copy link
Copy Markdown

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


38 regressions found
See build details here


Found 38 new test failures

Showing only first 15 new test failures

Test Explanation Runs Upstream
o.a.c.index.sai.cql.VectorCompaction100dTest.testOneToManyCompaction[version=ec enableNVQ=true] REGRESSION 🔴🔴 0 / 30
o.a.c.index.sai.cql.VectorSiftSmallTest.testRerankKZeroOrderMatchesFullPrecisionSimilarity[ec true] REGRESSION 🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=aa operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=aa operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=ca operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=ca operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=db operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=dc operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=ed operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnSkinnyTable[globalCurrentVersion=fa operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnWideTable[globalCurrentVersion=ba operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnWideTable[globalCurrentVersion=dc operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnWideTable[globalCurrentVersion=eb operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnWideTable[globalCurrentVersion=ec operation=CREATE] (compression) REGRESSION 🔵🔴 0 / 30
o.a.c.index.sai.cql.VersionSelectorTest.testBM25OnWideTable[globalCurrentVersion=fa operation=REBUILD] (compression) REGRESSION 🔵🔴 0 / 30

Found 7 known test failures

@adelapena adelapena merged commit a04cfb1 into main Jun 18, 2026
1 of 3 checks passed
@adelapena adelapena deleted the CNDB-16886-main branch June 18, 2026 15:05
scottfines pushed a commit that referenced this pull request Jun 19, 2026
… before EC (#2459)

Reject BM25 queries on `Index.validate(ReadCommand)` if the current
index version doesn't support them. That way they would be rejected on
the coordinator before execution, which is faster and produces a
descriptive error message, rather than the generic per-replica error
codes. The replicas still will do their own validation if the query gets
past the coordinator.
djatnieks pushed a commit that referenced this pull request Jun 23, 2026
… before EC (#2459)

Reject BM25 queries on `Index.validate(ReadCommand)` if the current
index version doesn't support them. That way they would be rejected on
the coordinator before execution, which is faster and produces a
descriptive error message, rather than the generic per-replica error
codes. The replicas still will do their own validation if the query gets
past the coordinator.
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.

3 participants