frankgh commented on code in PR #4936:
URL: https://github.com/apache/cassandra/pull/4936#discussion_r3580570498
##########
src/java/org/apache/cassandra/transport/ExceptionHandlers.java:
##########
@@ -76,7 +76,9 @@ public void exceptionCaught(final ChannelHandlerContext ctx,
Throwable cause)
if (ctx.channel().isOpen())
{
Predicate<Throwable> handler =
getUnexpectedExceptionHandler(ctx.channel(), false);
- ErrorMessage errorMessage = ErrorMessage.fromException(cause,
handler);
+ // No request in scope at the channel level; a
WrappedException cause carries the frame's stream id
+ // and overrides this 0 fallback, otherwise the channel is
torn down for fatal errors.
+ ErrorMessage errorMessage = ErrorMessage.fromException(cause,
0, handler);
Review Comment:
One thing to consider here is that we won't always overwrite the stream id.
I considered using the sentinel value, but this would trip the assertion during
the encoding. So we'll need to keep a valid value here.
The cases where the streamId will not be overwritten are:
- ExceptionHandlers.java:81 / PreV5Handlers.java:342 : This occurs during
channel negotiations (SSL/TLS handshake failures) and a stream ID is not
available
- Unexpected non-wrapped pipeline throwables where no frame/stream ID exists
at all
Everything else arrives in a wrapped exception and carries a real stream ID :
- CQLMessageHandler:517 : extracted.streamId() (best-effort)
- CQLMessageHandler:534 (oversized auth) : header.streamId
- CQLMessageHandler:725 (corrupt frame) : NO_REQUEST_STREAM_ID — explicit,
the one true context-free CQL path
I think keeping 0 for the `NO_REQUEST_STREAM_ID` is reasonable because the
only sources of missing stream ids are when an error occurs before a stream id
is available or when a a corrupt frame does not allow us to extract the stream
id.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
