You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by aljoscha <gi...@git.apache.org> on 2016/07/18 15:51:43 UTC
[GitHub] flink pull request #2266: [FLINK-4229] Do not start any Metrics Reporter by ...
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2266
[FLINK-4229] Do not start any Metrics Reporter by default
@rmetzger Is this what you had in mind when you mentioned that the JMXReporter should not be active by default because it starts a lot of RMI threads?
R: @zentol for review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aljoscha/flink metrics/no-default-reporter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2266.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 #2266
----
commit 68a0a8b1b822d9c69f74fe8208b7a798a486536d
Author: Aljoscha Krettek <al...@gmail.com>
Date: 2016-07-18T15:46:21Z
[FLINK-4229] Do not start any Metrics Reporter by default
----
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
Thanks for reviewing! I'll merge in a bit.
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2266
+1
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
Dammit, I thought about the doc but then forgot... changing.
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2266
This PR is a good time to make a slight change in `JMXReporter#open()`; making it use the `metrics.reporter.arguments` property instead of `metrics.jmx.port`. Since the special port property is not forwarded into the reporter configuration it will always use 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 pull request #2266: [FLINK-4229] Do not start any Metrics Reporter by ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2266#discussion_r71518447
--- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerMetricTest.java ---
@@ -50,9 +50,10 @@
public void testJobManagerMetricAccess() throws Exception {
Deadline deadline = new FiniteDuration(2, TimeUnit.MINUTES).fromNow();
Configuration flinkConfiguration = new Configuration();
-
+
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_CLASS, "org.apache.flink.metrics.reporter.JMXReporter");
flinkConfiguration.setString(ConfigConstants.METRICS_SCOPE_NAMING_JM_JOB, "jobmanager.<job_name>");
- flinkConfiguration.setString(ConfigConstants.METRICS_JMX_PORT, "9060-9075");
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_ARGUMENTS, "-port 9060-9075");
--- End diff --
The test seemed to also work with the single dash. \U0001f603 I'm changing.
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2266
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2266#discussion_r71527251
--- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerMetricTest.java ---
@@ -50,9 +50,10 @@
public void testJobManagerMetricAccess() throws Exception {
Deadline deadline = new FiniteDuration(2, TimeUnit.MINUTES).fromNow();
Configuration flinkConfiguration = new Configuration();
-
+
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_CLASS, "org.apache.flink.metrics.reporter.JMXReporter");
flinkConfiguration.setString(ConfigConstants.METRICS_SCOPE_NAMING_JM_JOB, "jobmanager.<job_name>");
- flinkConfiguration.setString(ConfigConstants.METRICS_JMX_PORT, "9060-9075");
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_ARGUMENTS, "-port 9060-9075");
--- End diff --
the test still worked since it defaulted back to the default port range; it looked for a property `port`, but only `-port` was defined. We set ports here to avoid port conflicts; there is no real hard requirement for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2266
The documentation for JMX wasn't updated; it still references `metrics.jmx.port`.
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
@zentol I addressed the stuff. Do you think this is alright 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 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 #2266: [FLINK-4229] Do not start any Metrics Reporter by ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2266#discussion_r71517800
--- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerMetricTest.java ---
@@ -50,9 +50,10 @@
public void testJobManagerMetricAccess() throws Exception {
Deadline deadline = new FiniteDuration(2, TimeUnit.MINUTES).fromNow();
Configuration flinkConfiguration = new Configuration();
-
+
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_CLASS, "org.apache.flink.metrics.reporter.JMXReporter");
flinkConfiguration.setString(ConfigConstants.METRICS_SCOPE_NAMING_JM_JOB, "jobmanager.<job_name>");
- flinkConfiguration.setString(ConfigConstants.METRICS_JMX_PORT, "9060-9075");
+ flinkConfiguration.setString(ConfigConstants.METRICS_REPORTER_ARGUMENTS, "-port 9060-9075");
--- End diff --
arguments are prefixed with a double dash `--`.
---
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 #2266: [FLINK-4229] Do not start any Metrics Reporter by default
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
@zentol I thought about that as well but wanted to keep the change to one issue. \U0001f603
I'll change it 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 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.
---