[ 
https://issues.apache.org/jira/browse/CASSANDRA-20984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18034205#comment-18034205
 ] 

Stefan Miklosovic edited comment on CASSANDRA-20984 at 10/30/25 3:54 PM:
-------------------------------------------------------------------------

I took a deeper look into the patch and I think that we should actually do 
something slightly different, I did it here (1).

The overall hardening seems to be the right way, but we need to test whole 
NettyStreamingConnectionFactory.connect (and what it returns / throws). It is 
not enough to just call initiateStream method. There is the complication in 
connect method of NettyStreamingConnectionFactory that it runs over all SSL 
fallbacks and additionally in inner loop of max connection attempts and so on. 
I think we need to handle Future and its result the correct way, check if it is 
success and so on, this is not done right now.

I also think that the change in OutboundConnection is not completely right 
because I am not sure what happens if this is not true: {{result.success() 
instanceof MessagingSuccess}}. Then what happens? It will just skip it and 
break? Then what? I think it is OK to put there simple assert but all changes 
should  go to streaming code imho, as we are trying to fix the reported issue 
in this ticket in the first place.

The patch is also clearly taking the inspiration from HandshakeTest, I am not 
completely sure if we should go over all of what it tests in StreamingTest. 
[~mck] what do you think about this? I see you have done CASSANDRA-18314 and 
was touching HandshakeTest.

(1) 
https://github.com/apache/cassandra/compare/trunk...smiklosovic:cassandra:CASSANDRA-20984-trunk?expand=1


was (Author: smiklosovic):
I took a deeper look into the patch and I think that we should actually do 
something slightly different, I did it here.

The overall hardening seems to be the right way, but we need to test whole 
NettyStreamingConnectionFactory.connect (and what it returns / throws). It is 
not enough to just call initiateStream method. There is the complication in 
connect method of NettyStreamingConnectionFactory that it runs over all SSL 
fallbacks and additionally in inner loop of max connection attempts and so on. 
I think we need to handle Future and its result the correct way, check if it is 
success and so on, this is not done right now.

I also think that the change in OutboundConnection is not completely right 
because I am not sure what happens if this is not true: {{result.success() 
instanceof MessagingSuccess}}. Then what happens? It will just skip it and 
break? Then what? I think it is OK to put there simple assert but all changes 
should  go to streaming code imho.

The patch is also clearly taking the inspiration from HandshakeTest, I am not 
completely sure if we should go over all of what it tests in StreamingTest. 
[~mck] what do you think about this? I see you have done CASSANDRA-18314 and 
was touching HandshakeTest.

(1) 
https://github.com/apache/cassandra/compare/trunk...smiklosovic:cassandra:CASSANDRA-20984-trunk?expand=1

> Fix java.lang.ClassCastException: Streaming Incompatible versions
> -----------------------------------------------------------------
>
>                 Key: CASSANDRA-20984
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-20984
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Consistency/Bootstrap and Decommission, 
> Consistency/Streaming
>            Reporter: Vivekanand Koya
>            Assignee: Vivekanand Koya
>            Priority: Normal
>             Fix For: 4.1.x, 5.0.x, 5.x
>
>          Time Spent: 2h 40m
>  Remaining Estimate: 0h
>
> On version mismatch or protocol incompatibility between the two communicating 
> nodes, cassandra server does not handle the errors correctly. There is no 
> proper error handling when ClassCastException occurs, since it is not 
> consistently reproducible. 
> In 
> [https://lists.apache.org/thread/ykkwhjdpgyqzw5xtol4v5ysz664bxxl3.,|https://lists.apache.org/thread/ykkwhjdpgyqzw5xtol4v5ysz664bxxl3.]
>  the "Stream failed" message points to a streaming operation, which is used 
> for processes like node repairs or adding new nodes (bootstrapping). I found 
> a similar Jira that was already raised - 
> https://issues.apache.org/jira/browse/CASSANDRA-19218.  Not sure what to do 
> with this JIRA. Looks like a duplicate.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to