Skip to content

Conversation

nikagra
Copy link

@nikagra nikagra commented Sep 24, 2025

Issue java-driver#596

repared statements invalidation used to be completely broken, you can read details on it here
Recently core merged PR that fixes problems for select statements.
After that PR, if driver supply SCYLLA_USE_METADATA_ID at the startup, when statements is prepared server hands out result queries metadata hash.
When driver executes this query it sends metadata id alongside with the request.
Server checks driver metadata id and what it has on its side, if there is any difference it will send new metadata and metadata id.
Driver suppose to pick it up and update metadata and metadata id on it's side, before deserializing response, solving issue with having outdated metadata on driver side.
Unfortunately it is solved only for SELECT statements, UPDATE/INSERT will be solved on separate occasion.

Core changes are in, we need to start working on driver side.

To Be Done:

  1. parse SCYLLA_USE_METADATA_ID protocol extension from SUPPORTED frame;
    • if present in SUPPORTED, send it in the STARTUP frame;
  2. if the extension is negotiated:
    • store the metadata ID, same way it is done for CQL v5
    • read METADATA_CHANGED (=3) flag in result metadata;
    • if the flag is set, read the new metadata ID (short bytes) and update it on prepare statement
  3. if received new result metadata that is different from the cached one:
    • use the new metadata for deserialization instead of the old one;
    • replace the old cached metadata with the new one.

Resolve skip metadata flag

Default value of skip metadata flag should be treated as safest option
Which means that if SCYLLA_USE_METADATA_ID was negotiated or CQL v5 is used then it is true, result metadata is skipped.
In other cases it is false.

Changes

  • Negotiating SCYLLA_USE_METADATA_ID extension during protocol initialization Options step.
  • Refactoring ProtocolFeature enum to ProtocolFeatures class enhancing isSUpportedBy method.
  • Introducing and storing in the channel ProtocolFeatureStore instance, which stores information about optional protocol features.
  • Enhancing Message to load ProtocolFeatureStore from channel and to later use it for encoding and decoding (in encoders and decoders).

@nikagra nikagra requested a review from dkropachev September 24, 2025 16:17
@nikagra nikagra self-assigned this Sep 24, 2025
/**
* Gets invoked when the listener is registered with a cluster, or at cluster startup if the
* listener was registered at initialization with {@link
* com.datastax.driver.core.Cluster#register(Host.StateListener)}.

Choose a reason for hiding this comment

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

Please move these kind of changes into a separate commit with proper message to it that explains why it is done

Copy link
Author

Choose a reason for hiding this comment

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

This should've been some automated change, but what tool has performed it. I will revert the changes

Choose a reason for hiding this comment

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

I still see them, please remove them from the PR.

Choose a reason for hiding this comment

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

Still see them

@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch from 55b46e4 to 559cd95 Compare September 29, 2025 10:25
@nikagra nikagra marked this pull request as ready for review October 3, 2025 17:14
@nikagra nikagra requested a review from Bouncheck October 3, 2025 17:16
@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch 3 times, most recently from 1349c00 to 2cbb7bf Compare October 10, 2025 11:26
@dkropachev dkropachev force-pushed the scylla-3.x-use-metadataId branch 2 times, most recently from e2971d2 to 82e78f0 Compare October 11, 2025 22:14
@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch from 3daac00 to ff4451f Compare October 13, 2025 13:10
@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch 4 times, most recently from 0d9e088 to 3b34d0c Compare October 13, 2025 21:18
/**
* Gets invoked when the listener is registered with a cluster, or at cluster startup if the
* listener was registered at initialization with {@link
* com.datastax.driver.core.Cluster#register(Host.StateListener)}.

Choose a reason for hiding this comment

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

I still see them, please remove them from the PR.

@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch from 70b2368 to ca5672c Compare October 14, 2025 12:01
@dkropachev dkropachev changed the title [#596] Initial implementation of SCYLLA_USE_METADATA_ID protocol fe… [#596] Initial implementation of SCYLLA_USE_METADATA_ID protocol feature negotiation Oct 14, 2025
@nikagra nikagra force-pushed the scylla-3.x-use-metadataId branch 2 times, most recently from 30edf34 to 9334677 Compare October 14, 2025 13:14
@dkropachev dkropachev force-pushed the scylla-3.x-use-metadataId branch from 288d8bc to c1d0287 Compare October 14, 2025 14:39
nikagra and others added 2 commits October 14, 2025 13:13
…ature negotiation. Introducing `ProtocolFeatureStore`.

[scylladb#596] Initial implementation of SCYLLA_USE_METADATA_ID feature.

[scylladb#596] Fixing NPE when accessing sharding info from empty ProtocolFeatureStore.

[scylladb#596] Storing `ProtocolFeatureStore` in `ProtocolEncoder`/`ProtocolDecoder`. Getting read of sharing it via `Channel` attributes.

[scylladb#596] Addressing missing `ProtocolFeatureStore` in a `statement` param of `executeAsync` to fix ITs.

[scylladb#596] Adjusting the way feature store is handled in `DefaultResultSetFuture`. Removing feature store from its constructor.

[scylladb#596] Moving channel attribute management for feature store to utility methods of `ProtocolFeatureStore`.

 [scylladb#596] Setting `Host`'s feature store separately from `Channel`. Code clean up

Apply suggestion from @dkropachev

Co-authored-by: Dmitry Kropachev <[email protected]>

 [scylladb#596] `ProtocolEncoder` clean up.
…validationTest`. Registering for closure resources implementing `AutoCloseable` interface in flaky tests class.

[scylladb#596] Improving Scylla test coverage in `PreparedStatementInvalidationTest`.

Apply suggestion from @dkropachev

Rearranging assertions and minor improvements in `should_never_update_statement_id_for_conditional_updates`

Co-authored-by: Dmitry Kropachev <[email protected]>
@dkropachev dkropachev force-pushed the scylla-3.x-use-metadataId branch from c1d0287 to 76a5354 Compare October 14, 2025 17:13
@dkropachev dkropachev merged commit fa6236e into scylladb:scylla-3.x Oct 14, 2025
6 of 7 checks passed
@nikagra nikagra deleted the scylla-3.x-use-metadataId branch October 15, 2025 13:51
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.

2 participants