[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 The reporter isn't closed and restarted; instead all metrics are being de-registered and then re-registered. This is necessary since the ID of the TM changes upon re-association with the JM. It would make sense to only un-register the metrics after we associate with a new JM; this should reduce the downtime of taskmanager metrics. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 Tested this vs. the current master and it works as expected. In the current master, the metrics are not updated after the TM disassociates from the JM. With this PR the reporter is closed, restarted and metrics are being reported again. In general, I'm wondering whether we should introduce an explicit suspended state where the metrics are still available and only reset after recovery. I was testing this with JMX and wondered whether it can be problematic for users if the reporter are down during JM recovery. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 OK that also explains why it didn't occur for me with the current release-1.1 branch. ;) --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 Sorry, while back-porting i realized this issue does not affect 1.1.4. I've adjusted the JIRA as such. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 Can you open a PR with a backport of this for 1.1.4 please? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 I tested this, but could not reproduce the reported erroneous behaviour. After the task manager reconnects to a new job manager, the metrics are still being reported. I tested this with JMX and Log4j reporters. The changes look reasonable to me though. @rmetzger @zentol Should we merge this for 1.1.4? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 The changes and the test look good to me. While waiting for Travis, I will test this with an HA cluster. We can merge this if everything succeeds. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 Actually, i don't know whether @rmetzger did review the functional changes; but that's only a single line. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 Someone has to review the test and then we're good to go IMO. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user uce commented on the issue: https://github.com/apache/flink/pull/2886 This has been marked as a blocker for 1.1.4. Is there a chance that we will get this in today? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 @rmetzger I've added a test case. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user zentol commented on the issue: https://github.com/apache/flink/pull/2886 That should be possible; I'll look into it. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #2886: [FLINK-5179] Correct TM MetricRegistry and TmMG lifecycle
Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/2886 I wonder if there is an easy way to add a test case for this? Maybe you could add a check to one of the HA tests? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
