You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jerryshao <gi...@git.apache.org> on 2014/11/25 08:24:15 UTC
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
GitHub user jerryshao opened a pull request:
https://github.com/apache/spark/pull/3444
[SPARK-4595][Core] Fix MetricsServlet not work issue
`MetricsServlet` handler should be added to the web UI after initialized by `MetricsSystem`, otherwise servlet handler cannot be attached.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jerryshao/apache-spark SPARK-4595
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3444.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 #3444
----
commit f779fe010f79ae70dc9e76cb9abd9edda6d2e16a
Author: jerryshao <sa...@intel.com>
Date: 2014-11-25T07:15:00Z
Fix MetricsServlet not work issue
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-64320122
[Test build #23822 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23822/consoleFull) for PR 3444 at commit [`f779fe0`](https://github.com/apache/spark/commit/f779fe010f79ae70dc9e76cb9abd9edda6d2e16a).
* This patch merges cleanly.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67381432
I tested this out and it looks good to me, so I'm going to merge this into `master`, `branch-1.2`, and `branch-1.1`. Thanks for fixing this!
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67220069
Ah, this was actually a pretty nasty bug since it seems to silently fail!
I'm fine merging this patch to fix this issue, but I wonder whether there are other changes that we could make to `MetricsSystem` in order to prevent these sorts of issues. For example, it looks like `MetricsSystem` has a bunch of public methods like `getServletHandlers` that don't indicate when they're safe to call (e.g. that they are only safe to call after the `MetricsSystem` has been started). Similarly, there are a bunch of public methods like `registerSources`, `buildRegistryName`, and `registerSinks()` that should be private.
I'd be happy to do this cleanup here, but maybe we should just merge this as-is then do the cleanup in a subsequent PR.
Actually, let's try this: I'll submit a PR to your PR which does the cleanup in `MetricsSystem.scala` in order to prevent these errors from occurring silently.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67269456
[Test build #24520 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24520/consoleFull) for PR 3444 at commit [`434d17e`](https://github.com/apache/spark/commit/434d17e11b3143b2572f39be79a8059cf4549552).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3444
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67217026
I'm finally getting around to reviewing this now. Is there a good way to test this? What's the error message / symptom thatI should look for in the buggy version of this code?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-64328090
[Test build #23822 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23822/consoleFull) for PR 3444 at commit [`f779fe0`](https://github.com/apache/spark/commit/f779fe010f79ae70dc9e76cb9abd9edda6d2e16a).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-64328095
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23822/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67382686
Actually, it looks like this issue doesn't affect 1.1.1, since I'm able to browse to `/metrics/json` after starting `spark-shell`. Therefore, I'm not merging this into `branch-1.1` (it would have tons of merge-conflicts there, too).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67269459
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24520/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67222433
I've opened a PR against this PR in order to add defensive checks to MetricsSystem, which would have prevented the issue fixed by this PR: https://github.com/jerryshao/apache-spark/pull/10
@jerryshao would be great if you could take a look at that and merge it into this PR branch if it looks fine to you (I've opened it against this branch).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67263058
[Test build #24520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24520/consoleFull) for PR 3444 at commit [`434d17e`](https://github.com/apache/spark/commit/434d17e11b3143b2572f39be79a8059cf4549552).
* This patch merges cleanly.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4595][Core] Fix MetricsServlet not work...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/3444#issuecomment-67262226
Thanks a lot Josh, I will merge into this PR and update the code :)
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org