Alex Petrov commented on CASSANDRA-15193:

The patch looks good to me, +1. 

One improvement we can do in {{Frame}}, when throwing a {{ProtocolException}}, 
is to use {{versionCap}} instead of {{CURRENT_VERSION}} in the error message.

I just have a couple of minor comments:

  * should we add a protocol negotiation test for cqlsh? It's a minor behaviour 
change, and we might want to ensure we preserve it.
  * should we stick to "max native protocol version" or to "max negotiable 
protocol version"? I think both have similar semantics / meaning, but it might 
be not obvious from the first glance.
  * {{maybeUpdateVersion}} can be private
  * in {{maybeUpdateVersion}}, we can avoid doubly-nested {{if}} by checking 
for {{!enforceV3Cap}} and returning if it happens. I wouldn't say it makes a 
huge change. Please feel free to ignore.
  * in {{Server.java}}, we can use {{ProtocolVersionLimit}} interface instead 
of {{ConfiguredLimit}} class, and make {{ConfiguredLimit}} package-private.
  * in {{ProtocolNegotiationTest}}, we can avoid calling 
{{setStaticLimitInConfig}} in {{finally}}, because it's already called in 

> Add ability to cap max negotiable protocol version
> --------------------------------------------------
>                 Key: CASSANDRA-15193
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15193
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Messaging/Client
>            Reporter: Sam Tunnicliffe
>            Assignee: Sam Tunnicliffe
>            Priority: Normal
>             Fix For: 3.0.x, 3.11.x
> 3.0 and native protocol V4 introduced a change to how PagingState is 
> serialized. Unfortunately that can break requests during upgrades: since 
> paging states are opaque, it's possible for a client to receive a paging 
> state encoded as V3 on a 2.1 node, and then send it to a 3.0 node on a V4 
> session. The version of the current session will be used to deserialize the 
> paging state, instead of the actual version used to serialize it, and the 
> request will fail.
> CASSANDRA-15176 solves half of this problem by enabling 3.0 nodes to 
> serialize mis-versioned PagingStates. To address the other side of the issue, 
> 2.1 nodes receiving V4 PagingStates, we can introduce a property to cap the 
> max native protocol version that the 3.0 nodes will negotiate with clients. 
> If we cap this to V3 during upgrades, no V4 connections will be established 
> and so no incompatible PagingStates will be sent to clients.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to