[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AnthonyTruchet commented on the issue: https://github.com/apache/spark/pull/14270 Hello @markgrover @vanzin ! As this is just a backport of your work, would you please consider reviewing 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 Sure. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user Ianwww commented on the issue:
https://github.com/apache/spark/pull/14270
Can this configuration be set in spark-defaults.conf as
spark.metrics.namespace=${spark.app.name}
@markgrover
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 Thanks a lot, @vanzin! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/14270 LGTM, merging to master. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62904/ Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62904 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62904/consoleFull)** for PR 14270 at commit [`8923c58`](https://github.com/apache/spark/commit/8923c58d324b8083ffb423d165f4707ec4395db2). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62903/ Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62903 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62903/consoleFull)** for PR 14270 at commit [`b9c9a7a`](https://github.com/apache/spark/commit/b9c9a7aa2b831247ae04d655f537223a02bc8440). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62904 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62904/consoleFull)** for PR 14270 at commit [`8923c58`](https://github.com/apache/spark/commit/8923c58d324b8083ffb423d165f4707ec4395db2). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62903 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62903/consoleFull)** for PR 14270 at commit [`b9c9a7a`](https://github.com/apache/spark/commit/b9c9a7aa2b831247ae04d655f537223a02bc8440). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 Fixed the nits, resolved the merge conflict. Shortened some test names. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/14270 Minor nit otherwise LGTM. Tests could use shorter names. Also, now there's a conflict... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62744/ Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62744 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62744/consoleFull)** for PR 14270 at commit [`3c8ea96`](https://github.com/apache/spark/commit/3c8ea966c5d3f356ad9fd4bead3fd2ced236c6bd). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62744 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62744/consoleFull)** for PR 14270 at commit [`3c8ea96`](https://github.com/apache/spark/commit/3c8ea966c5d3f356ad9fd4bead3fd2ced236c6bd). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 Ok, I have pushed changes to use the expansion capabilities brought in by SPARK-16272. Overall, I think it was a very good call to use that, so thanks for the suggestions! Would appreciate a review. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 ok, thanks, I will rely on changes in #14022 (SPARK-16272), now that it's been committed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user ericl commented on the issue: https://github.com/apache/spark/pull/14270 +1 on using the config refs provided by #14022 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/14270 It seems that the namespace must be the name of a configuration key, and cannot be an arbitrary string? That sounds a little limiting. You could change the code to use the value of the namespace config as the default, in case there's no config with the name the user provided, to allow some more flexibility. Or maybe if you instead rely on the changes in #14022, then the user would have even more flexibility. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 Tests passed! I'd really appreciate a review! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62549/ Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/14270 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62549 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62549/consoleFull)** for PR 14270 at commit [`605e690`](https://github.com/apache/spark/commit/605e690eefe905fa52979dde16f04bd208c8b219). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user markgrover commented on the issue: https://github.com/apache/spark/pull/14270 A few design choices I made along the way are: 1. Use a `SparkConf` property to control the namespace instead of a `MetricsConfig` property. The reason is that metrics config properties mean something particular and follow a particular pattern (see [here](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/metrics/MetricsSystem.scala#L54), for example). And, regardless of what we name this property, it doesn't follow that paradigm - the first part of the property name before the first dot doesn't represent an instance (like master, worker, executor, etc.) like it does for legit metrics properties. Also, the properties defined in `MetricsConfig` are obtained using the `getInstance()` call, where the instance is passed and again, such a retrieval doesn't really apply to this configuration property since it doesn't belong to an instance. I understand it's one more property in `SparkConf` but I really feel its belongs there, right by `spark.metrics.conf`. 2. Based on the previous point, this property shouldn't fall under the `spark.metrics.conf.` prefix. This is because if it does, it's mistakenly understood as a legit metrics property (see code [here](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/metrics/MetricsConfig.scala#L54), for example). 3. The proposed `spark.metrics.namespace` property allows one to specify an arbitrary property to be used as namespace. I did consider whitespacing it so users can use `spark.app.name` or `spark.app.id` only but then we'd have to deal with magic strings(app.name|app.id), and I didn't really feel inclined to do that. And, @ryan-williams who took a stab at the same issue #4632 made the same call and I agree with him. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #14270: [SPARK-5847][CORE] Allow for configuring MetricsSystem's...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/14270 **[Test build #62549 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62549/consoleFull)** for PR 14270 at commit [`605e690`](https://github.com/apache/spark/commit/605e690eefe905fa52979dde16f04bd208c8b219). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
