akalash commented on a change in pull request #17660:
URL: https://github.com/apache/flink/pull/17660#discussion_r748399598
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/BufferWritingResultPartition.java
##########
@@ -64,6 +64,8 @@
private TimerGauge backPressuredTimeMsPerSecond = new TimerGauge();
+ private long totalWrittenBytes;
Review comment:
I have thought about that. I think it is better to leave them duplicated
since right now we have `TaskIOMetricGroup` which handles the metrics the same
- `Counter` + `MeterView` but if we want to change it we need to do at least
the following:
- We need to adapt `MeterView` to receive value directly from some entity
rather than from `Counter`(or create a wrapper for counter). This is not a big
problem but anyway, this should be done.
- We need to initialize one particular metric differently since we won't be
able to create this metric in `TaskIOMetricGroup` constructor. This is a more
serious problem because it can lead to confusion if we start to initialize
metrics from one group differently in different places.
- We need to implement the same logic of `totalWrittenBytes` in other
subclasess of `ResultPartition` since right now it is just a helper for
calculation of metric in one specific implementation but if we decide to make
it as metric we need to have it everywhere(at least in
`SortMergeResultPartition`). Again, it is not a big problem but just an extra
complication.
So right now I think that it doesn't make sense to change anything in
`TaskIOMetricGroup` but in general, it is a good idea to think about our
metrics. Because the metric which collects data directly from the entity by
demand looks more flexible than the metric that passes the counter(or other
metric-related things) to the entity. But this is not today's discussion.
--
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]
