[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-13 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 Merging... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-13 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2210 Only had a small comment, otherwise +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-13 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 I've addressed your comments @zentol and introduced a `ProxyMetricGroup` which simply forwards all calls as you've suggested. --- If your project is set up for it, you can reply to this email

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-13 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 Ah ok, I see the underlying problem now. I will adapt my PR wrt your feedback. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2210 It gets an IOMG because in the past it would forward this group to its deserializers, for Input byte counting etc. . I didn't change it cause i forgot about my own convention on how to use the IOMG.

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 Why do we give the `StreamInputProcessor` a reference to the `IOMetricGroup` in the first place? Why not giving it the corresponding `TaskMetricGroup`? --- If your project is set up for it,

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2210 We could do that but it wouldn't be pretty. You would need a weird setValue() method on that Gauge that the StreamInputProcessor can use. Forwarding the register calls to the parent is the

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 The culprit is `StreamInputProcessor.java:220` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread tillrohrmann
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/2210 The problem was that the `currentLowWatermark` gauges weren't deregistered after the job terminated. This metric is registered by the `StreamInputProcessor` directly on the `IOMetricGroup` but

[GitHub] flink issue #2210: [FLINK-4167] [metrics] Close IOMetricGroup in TaskMetricG...

2016-07-07 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2210 Calling close() isn't necessary since no metric should ever be registered directly on the IOMetricGroup. This group is supposed to only contain pre-defined metrics that can be accessed from multiple