Skip to content

CNDB-12937: enable jvector 4; remove jvector 3 write support #1685

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
Apr 10, 2025

Conversation

michaeljmarshall
Copy link
Member

What is the issue

https://github.com/riptano/cndb/issues/12937. This is a follow up on #1676, which updated the jvector dependency but didn't update the jvector version in the actual graph builder.

What does this PR fix and why was it fixed

  • Adds jvector version configuration, defaulted to 4
  • Tests jvector versions 2 and 4 to ensure we can query sstables with both kinds of graphs
  • Makes the enable_hierarchy config backwards compatible by ignoring it if the jvector version is too low
  • Removes the ability to write FusedADC since it does not work with the latest jvector version (discovered when writing compatibility tests)
  • Does not remove FusedADC read path since there could be indexes based on it in production
  • Replaces call to deprecated graph.size() method with graph.size(0)

Copy link

github-actions bot commented Apr 9, 2025

Checklist before you submit for review

  • Make sure there is a PR 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 DataStax copyright header instead of the Apache License one

Copy link

sonarqubecloud bot commented Apr 9, 2025

@cassci-bot
Copy link

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


1 new test failure(s) in 4 builds
See build details here


Found 1 new test failures

Test Explanation Branch history Upstream history
o.a.c.u.b.BinLogTest.testTruncationReleasesLogS... regression 🔴🔵🔵🔴 🔵🔵🔵🔵🔵🔵🔵

Found 6 known test failures

@michaeljmarshall
Copy link
Member Author

Tests passed in https://github.com/riptano/cndb/pull/13596, merging now.

@michaeljmarshall michaeljmarshall merged commit 61f57a6 into main Apr 10, 2025
470 of 478 checks passed
@michaeljmarshall michaeljmarshall deleted the cndb-12937 branch April 10, 2025 17:31
djatnieks pushed a commit that referenced this pull request Apr 14, 2025
riptano/cndb#12937. This is a follow up on
#1676, which updated the
jvector dependency but didn't update the jvector version in the actual
graph builder.

* Adds jvector version configuration, defaulted to 4
* Tests jvector versions 2 and 4 to ensure we can query sstables with
both kinds of graphs
* Makes the `enable_hierarchy` config backwards compatible by ignoring
it if the jvector version is too low
* Removes the ability to write FusedADC since it does not work with the
latest jvector version (discovered when writing compatibility tests)
* Does not remove FusedADC read path since there could be indexes based
on it in production
* Replaces call to deprecated `graph.size()` method with `graph.size(0)`
djatnieks pushed a commit that referenced this pull request May 18, 2025
riptano/cndb#12937. This is a follow up on
#1676, which updated the
jvector dependency but didn't update the jvector version in the actual
graph builder.

* Adds jvector version configuration, defaulted to 4
* Tests jvector versions 2 and 4 to ensure we can query sstables with
both kinds of graphs
* Makes the `enable_hierarchy` config backwards compatible by ignoring
it if the jvector version is too low
* Removes the ability to write FusedADC since it does not work with the
latest jvector version (discovered when writing compatibility tests)
* Does not remove FusedADC read path since there could be indexes based
on it in production
* Replaces call to deprecated `graph.size()` method with `graph.size(0)`
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