Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-7069

Catch exceptions for each reporter separately

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.3.1, 1.4.0
    • Fix Version/s: 1.4.0, 1.3.2
    • Component/s: Metrics
    • Labels:
      None

      Description

      The metric system can be effectively disabled by a reporter that throws exceptions whenever it is notified of adding metrics.

      The reason is that the catching of exceptions isn't granular enough, as this peace of psude code shows:

      addMetric(metric):
      	try
      		for reporter in reporters:
      			reporter.addMetric(metric)
      		metricQueryService.addMetric(metric)
      	catch (e)
      		logError(e)	
      

      If a reporter throws an exception we never even attempt the other reporters, not notify the MQS which disabled metrics in the WebUI.

        Issue Links

          Activity

          Hide
          Zentol Chesnay Schepler added a comment -

          1.3: 8bfd1206f36470afd6d23798ec9f19b7e147df4b
          1.4: ee789583d3a7a9ee1bda5b9f3b0dd388279b9ad5

          Show
          Zentol Chesnay Schepler added a comment - 1.3: 8bfd1206f36470afd6d23798ec9f19b7e147df4b 1.4: ee789583d3a7a9ee1bda5b9f3b0dd388279b9ad5
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/4248

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/4248
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/4248

          merging.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/4248 merging.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on the issue:

          https://github.com/apache/flink/pull/4248

          Perfect, this LGTM now!

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4248 Perfect, this LGTM now!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/4248

          @aljoscha added a test

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/4248 @aljoscha added a test
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/4248

          sure, i can add a test.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/4248 sure, i can add a test.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on the issue:

          https://github.com/apache/flink/pull/4248

          The changes look good! 👍

          What about a test, though? 😅

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4248 The changes look good! 👍 What about a test, though? 😅
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

          https://github.com/apache/flink/pull/4248

          FLINK-7069 [metrics] Granular exception catching in MetricRegistry

          This PR makes the exception catching in the MetricRegistry more granular when adding or removing metrics.

          A reporter that throws an exception prevented other reporters, the MetricQueryService and the ViewUpdater from being notified about added or removed metrics.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/zentol/flink 7069

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/4248.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #4248


          commit 580bdaffdabfedd859e30ee7f37cf3f7983074f9
          Author: zentol <chesnay@apache.org>
          Date: 2017-07-03T14:49:01Z

          FLINK-7069 [metrics] Granular exception catching in MetricRegistry


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/4248 FLINK-7069 [metrics] Granular exception catching in MetricRegistry This PR makes the exception catching in the MetricRegistry more granular when adding or removing metrics. A reporter that throws an exception prevented other reporters, the MetricQueryService and the ViewUpdater from being notified about added or removed metrics. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 7069 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4248.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4248 commit 580bdaffdabfedd859e30ee7f37cf3f7983074f9 Author: zentol <chesnay@apache.org> Date: 2017-07-03T14:49:01Z FLINK-7069 [metrics] Granular exception catching in MetricRegistry

            People

            • Assignee:
              Zentol Chesnay Schepler
              Reporter:
              Zentol Chesnay Schepler
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development