[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-08 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723639969 ## On Volatile Reads and Performance Considerations I think `volatile` read assumption is completely correct. There is a common interpretation that this just means

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723222632 A thought on performance: There are some metrics that use `volatile` variables. So far we always avoided that. Even "infrequently accessed" fields, like watermarks,

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723218943 The PR adds a new `numBytesIn` metric to the `OperatorIOMetricGroup`. That means all operators now show that metric. But the metric is zero for the majority of operators

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723208144 @zentol Curious what your take is here. This is an automated message from the Apache Git Service. To

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723207966 Thanks for implementing this. At a first glance, I think moving the `MetricNames` to `flink-core` does not work too well. The metric names don't have a meaning by

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723196124 Taking a look now. @becketqin The code currently fails with `NullPointerException` in some tests.

[GitHub] [flink] StephanEwen commented on pull request #13920: [FLINK-19743] Add metric definitions in FLIP-33 and report some of them by default.

2020-11-06 Thread GitBox
StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723189562 Sorry to be late to the game here, but could you share a bit more information on what the original setup was? Specifically, what was your checkpoint storage system that