You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by alexmnyc <gi...@git.apache.org> on 2017/09/12 18:53:16 UTC
[GitHub] spark pull request #19210: Fix Graphite re-connects for Graphite instances b...
GitHub user alexmnyc opened a pull request:
https://github.com/apache/spark/pull/19210
Fix Graphite re-connects for Graphite instances behind ELB's or auto-scaled load balancers
…resolve hosts behind a CNAME with re-tried DNS lookups
Upgrade codahale metrics library so that Graphite constructor can re-resolve hosts behind a CNAME with re-tried DNS lookups. When Graphite is deployed behind an ELB, ELB may change IP addresses based on auto-scaling needs. Using current approach yields Graphite usage impossible, fixing for that use case
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/alexmnyc/spark patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19210.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 #19210
----
commit 4693801be78418f1939e07a0d8dc482e3e1efda2
Author: alexmnyc <pr...@alexandermarkham.com>
Date: 2017-09-12T18:49:40Z
Upgrade codahale metrics library so that Graphite constructor can re-resolve hosts behind a CNAME with re-tried DNS lookups
Upgrade codahale metrics library so that Graphite constructor can re-resolve hosts behind a CNAME with re-tried DNS lookups. When Graphite is deployed behind an ELB, ELB may change IP addresses based on auto-scaling needs. Using current approach yields Graphite usage impossible, fixing for that use case
commit 5e2daadaa65d4e5f1988937fed050ee7905a42ea
Author: alexmnyc <pr...@alexandermarkham.com>
Date: 2017-09-12T18:52:41Z
Force coda-hale Graphite re-try DNS resolution for Graphite instances behind auto-scaled load balancers
This fix is introduced in codahale 3.1.5 - https://github.com/dropwizard/metrics/compare/v3.1.2...v3.1.5#diff-6916c85d2dd08d89fe771c952e3b8512R120
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81875/testReport)** for PR 19210 at commit [`0458123`](https://github.com/apache/spark/commit/0458123c1b3827f8b4b55eeb8bd5f7dbc749a4aa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81770 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81770/testReport)** for PR 19210 at commit [`8e982c7`](https://github.com/apache/spark/commit/8e982c7d450498580ab857baeed2650488ea1837).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81802/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
BTW, can you please create a JIRA, and fix the PR title like other PRs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
Jenkins, retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81875/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19210: [SPARK-22030][CORE] GraphiteSink fails to re-conn...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19210
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81754/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19210: Fix Graphite re-connects for Graphite instances b...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19210#discussion_r139045423
--- Diff: core/src/main/scala/org/apache/spark/metrics/sink/GraphiteSink.scala ---
@@ -69,7 +69,7 @@ private[spark] class GraphiteSink(val property: Properties, val registry: Metric
val graphite = propertyToOption(GRAPHITE_KEY_PROTOCOL).map(_.toLowerCase(Locale.ROOT)) match {
case Some("udp") => new GraphiteUDP(new InetSocketAddress(host, port))
- case Some("tcp") | None => new Graphite(new InetSocketAddress(host, port))
+ case Some("tcp") | None => new Graphite(host, port)
--- End diff --
Yes, that's what I mean, I'm not sure if it is required since I'm not familiar with this Graphite sink.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on the issue:
https://github.com/apache/spark/pull/19210
@jerryshao No problem. I don't have a JIRA account for Apache though. How do I go about getting one?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19210: Fix Graphite re-connects for Graphite instances b...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on a diff in the pull request:
https://github.com/apache/spark/pull/19210#discussion_r138924426
--- Diff: core/src/main/scala/org/apache/spark/metrics/sink/GraphiteSink.scala ---
@@ -69,7 +69,7 @@ private[spark] class GraphiteSink(val property: Properties, val registry: Metric
val graphite = propertyToOption(GRAPHITE_KEY_PROTOCOL).map(_.toLowerCase(Locale.ROOT)) match {
case Some("udp") => new GraphiteUDP(new InetSocketAddress(host, port))
- case Some("tcp") | None => new Graphite(new InetSocketAddress(host, port))
+ case Some("tcp") | None => new Graphite(host, port)
--- End diff --
@jerryshao Do you mean change Graphite constructor for case "udp" ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
@HyukjinKwon would you please help to trigger the Jenkins? Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on the issue:
https://github.com/apache/spark/pull/19210
@jerryshao done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19210
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81770 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81770/testReport)** for PR 19210 at commit [`8e982c7`](https://github.com/apache/spark/commit/8e982c7d450498580ab857baeed2650488ea1837).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
You should also update the the files under `dev/deps`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/19210
Sure.
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19210: Fix Graphite re-connects for Graphite instances b...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on a diff in the pull request:
https://github.com/apache/spark/pull/19210#discussion_r139046210
--- Diff: core/src/main/scala/org/apache/spark/metrics/sink/GraphiteSink.scala ---
@@ -69,7 +69,7 @@ private[spark] class GraphiteSink(val property: Properties, val registry: Metric
val graphite = propertyToOption(GRAPHITE_KEY_PROTOCOL).map(_.toLowerCase(Locale.ROOT)) match {
case Some("udp") => new GraphiteUDP(new InetSocketAddress(host, port))
- case Some("tcp") | None => new Graphite(new InetSocketAddress(host, port))
+ case Some("tcp") | None => new Graphite(host, port)
--- End diff --
Looks like there is a constructor accepting hostname directly for UDP. I'll add that as well.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
Jenkins, retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on the issue:
https://github.com/apache/spark/pull/19210
@jerryshao done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19210: Fix Graphite re-connects for Graphite instances b...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19210#discussion_r138879962
--- Diff: core/src/main/scala/org/apache/spark/metrics/sink/GraphiteSink.scala ---
@@ -69,7 +69,7 @@ private[spark] class GraphiteSink(val property: Properties, val registry: Metric
val graphite = propertyToOption(GRAPHITE_KEY_PROTOCOL).map(_.toLowerCase(Locale.ROOT)) match {
case Some("udp") => new GraphiteUDP(new InetSocketAddress(host, port))
- case Some("tcp") | None => new Graphite(new InetSocketAddress(host, port))
+ case Some("tcp") | None => new Graphite(host, port)
--- End diff --
Do we need to change here also?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
LGTM, let me retest this again.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81802 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81802/testReport)** for PR 19210 at commit [`0458123`](https://github.com/apache/spark/commit/0458123c1b3827f8b4b55eeb8bd5f7dbc749a4aa).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81754 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81754/testReport)** for PR 19210 at commit [`8e982c7`](https://github.com/apache/spark/commit/8e982c7d450498580ab857baeed2650488ea1837).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81754 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81754/testReport)** for PR 19210 at commit [`8e982c7`](https://github.com/apache/spark/commit/8e982c7d450498580ab857baeed2650488ea1837).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81875/testReport)** for PR 19210 at commit [`0458123`](https://github.com/apache/spark/commit/0458123c1b3827f8b4b55eeb8bd5f7dbc749a4aa).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19210
**[Test build #81802 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81802/testReport)** for PR 19210 at commit [`0458123`](https://github.com/apache/spark/commit/0458123c1b3827f8b4b55eeb8bd5f7dbc749a4aa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by alexmnyc <gi...@git.apache.org>.
Github user alexmnyc commented on the issue:
https://github.com/apache/spark/pull/19210
@jerryshao thanks. it's done.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: Fix Graphite re-connects for Graphite instances behind E...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19210
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81770/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19210: [SPARK-22030][CORE] GraphiteSink fails to re-connect to ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19210
LGTM, merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org