You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by uce <gi...@git.apache.org> on 2016/07/03 14:20:59 UTC

[GitHub] flink pull request #2193: [FLINK-4145] [core] Configure port range for JMXRe...

GitHub user uce opened a pull request:

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

    [FLINK-4145] [core] Configure port range for JMXReporterTest

    I saw multiple failures of the JmxReporterTest most likely due to a port conflicts. The test relies on the default JMX reporter port range, which spans 5 ports. Running on Travis with multiple concurrent builds and bad timings, this can lead to port conflicts.
    
    Some example failed runs:
    https://s3.amazonaws.com/archive.travis-ci.org/jobs/141999066/log.txt (one out of 5 jobs failed)
    https://travis-ci.org/uce/flink/builds/141917901 (all 5 jobs failed)
    
    I propose to take the fork number into account (like the forkable Flink testing cluster) and configure a larger port range.
    
    Can you review this, @zentol?


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

    $ git pull https://github.com/uce/flink 4145-jmx_test

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

    https://github.com/apache/flink/pull/2193.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 #2193
    
----
commit dbe217a1bd9eaa9a98c7ae9800fec8d2af26e2f3
Author: Ufuk Celebi <uc...@apache.org>
Date:   2016-07-03T14:08:17Z

    [FLINK-4145] [core] Configure port range for JMXReporterTest

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2193: [FLINK-4145] [core] Configure port range for JMXReporterT...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/2193
  
    I believe the underlying issue is that there are a bunch of tests that just don't close their ports properly, as for quite a few tests a MetricRegistry is fired up silently in the background.
    
    To circumvent this we don't even need a random range or a greater one, just once that is distinct from the default port range.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2193: [FLINK-4145] [core] Configure port range for JMXReporterT...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/2193
  
    the default port range is 9010-9025, that's 15 not 5 ports.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2193: [FLINK-4145] [core] Configure port range for JMXRe...

Posted by uce <gi...@git.apache.org>.
Github user uce closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2193: [FLINK-4145] [core] Configure port range for JMXRe...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2193#discussion_r69421710
  
    --- Diff: flink-core/src/test/java/org/apache/flink/metrics/reporter/JMXReporterTest.java ---
    @@ -285,4 +289,23 @@ public long getMin() {
     			};
     		}
     	}
    +
    +	/**
    +	 * Sets the JMX port range config key based on the fork number.
    +	 * @param config Configuration instance to configure
    +	 * @return Passed configuration instance
    +	 */
    +	private static Configuration setJmxPortRange(Configuration config) {
    +		int forkNumber = 0;
    +		try {
    +			forkNumber = Integer.parseInt(System.getProperty("forkNumber"));
    +		} catch (NumberFormatException ignored) {
    +		}
    +
    +		int minPort = 9000 + forkNumber * 100;
    --- End diff --
    
    I think the fort number thing was non sensical as it is only set for integration 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2193: [FLINK-4145] [core] Configure port range for JMXRe...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2193#discussion_r69397170
  
    --- Diff: flink-core/src/test/java/org/apache/flink/metrics/reporter/JMXReporterTest.java ---
    @@ -285,4 +289,23 @@ public long getMin() {
     			};
     		}
     	}
    +
    +	/**
    +	 * Sets the JMX port range config key based on the fork number.
    +	 * @param config Configuration instance to configure
    +	 * @return Passed configuration instance
    +	 */
    +	private static Configuration setJmxPortRange(Configuration config) {
    +		int forkNumber = 0;
    +		try {
    +			forkNumber = Integer.parseInt(System.getProperty("forkNumber"));
    +		} catch (NumberFormatException ignored) {
    +		}
    +
    +		int minPort = 9000 + forkNumber * 100;
    --- End diff --
    
    what does the forkNumber bring us here? wouldn't it be better to use it for the mapPort to increase the range of ports?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2193: [FLINK-4145] [core] Configure port range for JMXReporterT...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:

    https://github.com/apache/flink/pull/2193
  
    I'm closing this after an offline chat with @zentol who will fix this properly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2193: [FLINK-4145] [core] Configure port range for JMXReporterT...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/2193
  
    I would suggest using a random port range.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---