[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-23 Thread zentol
Github user zentol closed the pull request at: https://github.com/apache/flink/pull/1947 --- 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 if the feature is

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-22 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220850557 I did a pass over the code and committed the result: Manually merged in 003ce18efc0249fae874e56c3df6acf19f5f2429 and

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-19 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220319335 Let me grab the token for this one. There are a few things still, like resource leaks, in this code. I'll pass you back the token as soon as I am done... ---

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220112639 renaming the module is a good idea --- 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

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220099057 How about renaming the maven module from `flink-metrics` to `flink-metric-reporters`? The metrics systems itself is part of `flink-core` after all... --- If your

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220087815 I've had an offline chat with @StephanEwen and as a result updated the PR. List of changes: * dropped Meters, Histograms and Timers * we weren't

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220015012 I would like to take an iteration on this, make some changes on top of this, and open a new pull request afterwards. --- If your project is set up for it, you can

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-220014866 Few followups are actually needed before merging this: 1. We need to remove the example metrics 2. Conflicting metrics names should not result

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-219964437 Hmm, seeing some local test failures after the re-base. Have to look into this. Typical message is `java.lang.IllegalArgumentException: This group already

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-18 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-219959102 Tests were good! Rebased again, re-running tests, will merge after that. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-17 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-219864781 I rebased the branch, waiting for Travis to give a green light... --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-17 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-219849631 The code in this PR is the most up-to-date. --- 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

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-17 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-219836974 This looks pretty good now. I would like to get this in soon, now that the test are passing. Let's iterate over it on the master. This needs a rebase

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-13 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-218983234 The tests should now pass. I've fixed the following issues: * several examples had non-transient Metric fields * removed one failing example

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-10 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-218153465 I'm currently aware of the following issues: * quite a few tests in flink-runtime fail; this is mostly due to a missing integration into the MockEnvironments. Fix

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-10 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-218146940 Looks nice, I am trying it out now --- 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

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-09 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217972596 I have updates this PR; it now contains a new feature: configurable system scope. It essentially allows users to decide how Flink entities(Taskmanager, jobs, tasks and

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-07 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217636194 @ankitcha you could write your own reporter that simply wraps the reporters you want to use, simply forwarding the calls to all of them. this should actually be really

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-06 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217389510 Okay. I think we need to support multiple instances of the same job on a TaskManager. --- If your project is set up for it, you can reply to this email and have your

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-06 Thread zentol
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217368732 @rmetzger you are correct that you're job failed since the previous one wasn't cleaned up yet. Should you try to run 2 identical jobs in parallel it will fail, since 2

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-05 Thread ankitcha
Github user ankitcha commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217240628 Guys, Thanks you for this awesome addition to Flink. I was just wondering if there is a way to configure multiple reporters as well? --- If your project is

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-05 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217214295 I'm just assuming its the missing "close()" call's I've commented already causing this issue. I just tried adding some custom metrics: ```java new

[GitHub] flink pull request: [FLINK-1502] [WIP] Basic Metric System

2016-05-05 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1947#issuecomment-217213101 Is it possible that submitting the same job two times doesn't work? ``` ./bin/flink run ./examples/streaming/SocketWindowWordCount.jar --port 54323