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

InputGateMetrics#refreshAndGetMin returns Integer.MAX_VALUE for local channels

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.3.0, 1.4.0
    • Fix Version/s: 1.3.0, 1.4.0
    • Component/s: Metrics, Network
    • Labels:
      None

      Description

      The InputGateMetrics#refreshAndGetMin returns Integer.MAX_VALUE when working with LocalChannels. In this case it should return 0 instead.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

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

          FLINK-6586 InputGateMetrics return 0 as minimum for local channels

          Should be merged for 1.3 and 1.4.

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

          $ git pull https://github.com/zentol/flink 6586_network_min

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

          https://github.com/apache/flink/pull/3907.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 #3907


          commit c74701c17718e1260b5d2b3e96ab09c01f9d3000
          Author: zentol <chesnay@apache.org>
          Date: 2017-05-15T11:56:06Z

          FLINK-6586 InputGateMetrics return 0 as min for local channels


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/3907 FLINK-6586 InputGateMetrics return 0 as minimum for local channels Should be merged for 1.3 and 1.4. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 6586_network_min Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3907.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 #3907 commit c74701c17718e1260b5d2b3e96ab09c01f9d3000 Author: zentol <chesnay@apache.org> Date: 2017-05-15T11:56:06Z FLINK-6586 InputGateMetrics return 0 as min for local channels
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on the issue:

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

          Does an `InputGate` contain both local and remote `InputChannel` such that the minimum positive value might be more desirable in that case?

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3907 Does an `InputGate` contain both local and remote `InputChannel` such that the minimum positive value might be more desirable in that case?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          That's a great point, haven't considered that. It may be easier to just compare the final result with `Integer.MAX_VALUE` and return `0` if the match.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3907 That's a great point, haven't considered that. It may be easier to just compare the final result with `Integer.MAX_VALUE` and return `0` if the match.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          Updated the PR.

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

          Github user greghogan commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3907#discussion_r117479119

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/InputGateMetrics.java —
          @@ -86,6 +86,9 @@ int refreshAndGetMin() {
          }
          }

          + if (min == Integer.MAX_VALUE) { // in case all channels are local
          — End diff –

          Is it worth removing the earlier check for `channels.isEmpty()` and merging the comments?

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/3907#discussion_r117479119 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/InputGateMetrics.java — @@ -86,6 +86,9 @@ int refreshAndGetMin() { } } + if (min == Integer.MAX_VALUE) { // in case all channels are local — End diff – Is it worth removing the earlier check for `channels.isEmpty()` and merging the comments?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3907#discussion_r117481827

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/InputGateMetrics.java —
          @@ -86,6 +86,9 @@ int refreshAndGetMin() {
          }
          }

          + if (min == Integer.MAX_VALUE) { // in case all channels are local
          — End diff –

          yes.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3907#discussion_r117481827 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/InputGateMetrics.java — @@ -86,6 +86,9 @@ int refreshAndGetMin() { } } + if (min == Integer.MAX_VALUE) { // in case all channels are local — End diff – yes.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          updated.

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

          Github user greghogan commented on the issue:

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

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3907 +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          merging.

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

          Github user asfgit closed the pull request at:

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

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

          1.3: c3ab5c8253b32bddc3fb9bf0c1085813e7f97e2f
          1.4: 17ec6f020b779efe9152456f4ef33f6f802e4f67

          Show
          Zentol Chesnay Schepler added a comment - 1.3: c3ab5c8253b32bddc3fb9bf0c1085813e7f97e2f 1.4: 17ec6f020b779efe9152456f4ef33f6f802e4f67

            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