isaacreath commented on PR #2058: URL: https://github.com/apache/cassandra/pull/2058#issuecomment-1373865183
@smiklosovic After thinking about this issue for a little bit, @pauloricardomg and I came up with a different approach. We moved the logic which increments the incoming / outgoing bytes metrics from the `StreamSession` to a dedicated class called the `FileStreamMetricsListener`. `FileStreamMetricsListener` objects will then be created by each `CassandraIncomingFile` and `CassandraOutgoingFile`. These then pass the `FileStreamMetricsListener` to the appropriate stream reader / writer. As those reader / writers transfer bytes, `FileStreamMetricsListener::onBytesTransferred` will be called to update the metrics. When file transfer is complete, we call `FileStreamMetricsListener::onStreamSuccessful` to verify that we've tracked the size appropriately (thus ensuring that any new stream reader / writers are calling `onBytesTransferred`). This approach addresses the concerns of having an ever-growing map of `ProgressInfo` objects and it neatly encapsulates the logic for updating these counter objects. There are some aspects of this approach of which I am not a fan, namely the fact that there is little ensuring that new Stream reader / writer objects actually create a `FileStreamMetricsListener` and update it properly; but, from looking at the code, there is some refactoring which needs to be done to unify all of the stream reader / writer classes in a single hierarchy that should probably be handled outside the scope of this ticket. -- 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]
