Skip to content

Skip DSE versions during protocol negotiation - #985

Open
arnabnandy7 wants to merge 1 commit into
scylladb:scylla-4.xfrom
arnabnandy7:feature/skip-dse-protocol-negotiation
Open

Skip DSE versions during protocol negotiation#985
arnabnandy7 wants to merge 1 commit into
scylladb:scylla-4.xfrom
arnabnandy7:feature/skip-dse-protocol-negotiation

Conversation

@arnabnandy7

Copy link
Copy Markdown

Summary

Fixes #969 by excluding DSE protocol versions from automatic protocol negotiation.

The initial negotiation sequence now uses standard Cassandra protocol versions:

V5 → V4 → V3

This avoids rejected DSE_V2 and DSE_V1 connection attempts when starting a session against ScyllaDB.

Explicitly configured DSE protocol versions remain supported.

Changes

  • Start automatic negotiation with the highest non-beta OSS protocol version.
  • Add unit coverage for the negotiation starting version.
  • Update native protocol documentation to describe the new behavior.
  • Add a changelog entry.

Testing

mvn -pl core -Dtest=DefaultProtocolVersionRegistryTest test

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Automatic protocol negotiation now selects only OSS protocol versions. highestNonBeta() returns V5 instead of considering DSE versions. DSE protocol versions remain available when explicitly forced. The DSE compatibility documentation reflects standard protocol negotiation and forced DSE_V1 or DSE_V2 selection for DSE-specific features. A changelog entry and a configuration reference update document the changes.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The configuration reference link update is unrelated to the linked issue and stated protocol negotiation objectives. Move the configuration reference link correction to a separate pull request, or document its required scope and relationship to this change.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the primary change: excluding DSE versions from automatic protocol negotiation.
Description check ✅ Passed The description accurately explains the negotiation change, preserved DSE support, documentation updates, changelog entry, and test coverage.
Linked Issues check ✅ Passed The changes address issue #969 by removing automatic DSE probes while preserving OSS negotiation and explicit DSE protocol selection.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
@arnabnandy7
arnabnandy7 force-pushed the feature/skip-dse-protocol-negotiation branch from 8244552 to 6584965 Compare August 3, 2026 18:23
@arnabnandy7

Copy link
Copy Markdown
Author

Fixed CI failure due to documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
manual/core/native_protocol/README.md-55-66 (1)

55-66: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document DSE 7.0+ in the compatibility table.

DefaultProtocolVersionRegistry handles DSE 7.0+ separately and allows OSS V5, but this table stops at DSE 6.8 while presenting the DSE negotiation matrix. Add a 7.0+ | v5 row or state that the table excludes DSE 7.0+.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@manual/core/native_protocol/README.md` around lines 55 - 66, Update the DSE
compatibility table in the native protocol documentation to include a 7.0+ entry
negotiated as v5, reflecting DefaultProtocolVersionRegistry behavior; keep the
existing rows and surrounding explanation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Other comments:
In `@manual/core/native_protocol/README.md`:
- Around line 55-66: Update the DSE compatibility table in the native protocol
documentation to include a 7.0+ entry negotiated as v5, reflecting
DefaultProtocolVersionRegistry behavior; keep the existing rows and surrounding
explanation unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: ab80d0ec-2a75-4207-a776-a70601932d3b

📥 Commits

Reviewing files that changed from the base of the PR and between 8244552 and 6584965.

📒 Files selected for processing (5)
  • changelog/README.md
  • core/src/main/java/com/datastax/oss/driver/internal/core/DefaultProtocolVersionRegistry.java
  • core/src/test/java/com/datastax/oss/driver/internal/core/DefaultProtocolVersionRegistryTest.java
  • manual/core/configuration/reference/README.rst
  • manual/core/native_protocol/README.md

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.

Protocol negotiation probes DSE_V2/DSE_V1/V5 before V4 on startup against ScyllaDB

2 participants