isaacreath commented on code in PR #2058:
URL: https://github.com/apache/cassandra/pull/2058#discussion_r1065066562
##########
src/java/org/apache/cassandra/db/streaming/CassandraOutgoingFile.java:
##########
@@ -172,10 +174,11 @@ public void write(StreamSession session,
StreamingDataOutputPlus out, int versio
out.flush();
CassandraStreamWriter writer = header.isCompressed() ?
- new
CassandraCompressedStreamWriter(sstable, header, session) :
- new CassandraStreamWriter(sstable,
header, session);
+ new
CassandraCompressedStreamWriter(sstable, header, session,
fileStreamMetricsListener) :
+ new CassandraStreamWriter(sstable,
header, session, fileStreamMetricsListener);
writer.write(out);
}
+ fileStreamMetricsListener.onStreamSuccessful();
Review Comment:
I tried making the change to `isStreamSuccessful` from `onStreamSuccessful`
and it seems to make the `FileStreamMetricsListener` a very leaky abstraction
to provide a proper error message and we end up duplicating the checking logic
between the `CassandraIncomingFile` and `CassandraOutgoingFile` which I'm not a
huge fan of (and again, seems like a leaky abstraction).
I could instead throw an `IOException` instead of running the assert; but,
like you say, that feels strange as no IO is actually happening in
`onStreamSuccessful`.
--
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]
