Cassandra 15665 trunk 15666 #586
Conversation
src/java/org/apache/cassandra/streaming/async/StreamingInboundHandler.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/streaming/async/StreamingInboundHandler.java
Outdated
Show resolved
Hide resolved
| */ | ||
| public class StreamMessageHeader | ||
| { | ||
| public static FileMessageHeaderSerializer serializer = new FileMessageHeaderSerializer(); | ||
|
|
||
| public final TableId tableId; | ||
| public UUID planId; | ||
| // it tells us if the file was sent by a follower | ||
| public final boolean isFollower; |
There was a problem hiding this comment.
At the level of the message isFollower is a confusing name. sendByFollower or fromFollower would make more sense to me.
There was a problem hiding this comment.
updated to sendByFollower, the new name is clearer in the context of stream message.
| @@ -176,17 +176,10 @@ public void removeNotificationListener(NotificationListener listener, Notificati | |||
| return notifier.getNotificationInfo(); | |||
| } | |||
|
|
|||
| public StreamSession findSession(InetAddressAndPort peer, UUID planId, int sessionIndex) | |||
| public StreamSession findSession(InetAddressAndPort peer, UUID planId, int sessionIndex, boolean sentByFollower) | |||
There was a problem hiding this comment.
The argument name 'sentByFollower' makes sense in the context of the IncomingStreamMessage but not so much in the context of this method. In my opinion we should either rename the parameter to something like searchInitiatorSessions or add some comment to explain the logic.
There was a problem hiding this comment.
👍 updated to searchInitiatorSessions
| if (streamSession == null) | ||
| throw new IllegalStateException(createLogTag(null, channel) + " no session found for message " + message); |
There was a problem hiding this comment.
Based on my understanding, calling getOrCreateSession on StreamInitMessage or IncomingStreamMessage is guaranty to return a StreamSession and calling getOrCreateSession on another type of message would throw an UnsupportedOperationException. By consequence, it seems that this null check is useless. Am I missing something?
There was a problem hiding this comment.
it shouldn't be null. I will remove it.
jasonstack
force-pushed
the
CASSANDRA-15665-trunk-15666
branch
from
6265a7f
to
2928a88
Compare
|
rebased and applied review feedback. CI Running: https://circleci.com/workflow-run/947320b4-3c52-4113-91ab-36d622a45e23 |
Circle-CI