[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-17 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-67386266 I've merged this into `branch-1.2`, so this fix will be included in Spark 1.2.1. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/3570 --- 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 enab

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66415593 Thanks for updating the description. This looks good to me, so I'm going to merge this into `master`, `branch-1.0`, and `branch-1.1` (and I'll tag it for a post-releas

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66399588 sorry, must have accidentally hit cancel instead of comment the first time. Should be set now. --- If your project is set up for it, you can reply to this email and

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66398560 I thought I'd done so, it looks like it lost my changes I'll fix that asap --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66398459 LGTM. Just in case you missed my earlier comment, are you still planning to update the PR description to reflect the actual changes vs. the ones you had planned? ---

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66391021 [Test build #24274 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24274/consoleFull) for PR 3570 at commit [`a581f3f`](https://gith

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66391029 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66383538 @kayousterhout I'm glad to see that changing it to `private[spark]` fixed things without requiring a MiMa exclude for the `private -> private[spark]` change. This is g

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66383350 [Test build #24274 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24274/consoleFull) for PR 3570 at commit [`a581f3f`](https://githu

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66380241 comment fixed. I'm trying to test the MiMa related changes to see if they work, and having problems running mima on my machine. I'll probably just push them in the

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request: https://github.com/apache/spark/pull/3570#discussion_r21573219 --- Diff: core/src/main/scala/org/apache/spark/Accumulators.scala --- @@ -281,7 +282,9 @@ object AccumulatorParam { private object Accumulators {

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread nkronenfeld
Github user nkronenfeld commented on a diff in the pull request: https://github.com/apache/spark/pull/3570#discussion_r21572834 --- Diff: core/src/main/scala/org/apache/spark/Accumulators.scala --- @@ -281,7 +282,9 @@ object AccumulatorParam { private object Accumulators {

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread kayousterhout
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66360844 To fix the MiMA problem, can you instead make Accumulators a private[spark] object? No one I've asked seems to understand what "private" even means in this context

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread andrewor14
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66357628 LGTM, nice catch. --- 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 fe

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66346390 This looks good to me. If you don't mind, could you update the pull request description to more accurately describe the change that we're actually committing? This is

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-09 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request: https://github.com/apache/spark/pull/3570#discussion_r21557013 --- Diff: core/src/main/scala/org/apache/spark/Accumulators.scala --- @@ -281,7 +282,9 @@ object AccumulatorParam { private object Accumulators {

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-08 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-66238332 Any word on 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 f

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65883848 oh, a note for when you're reviewing - I didn't move the clear call, I just added a second one; I saw no particular harm in leaving the old one there too, just in cas

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65883762 great... I think outside the mima issue, it should be all set, unless I can figure out a way to unit test it. So far, my best methods of testing it involve instrumen

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65883736 No, I'd leave it. I just thought I'd mention it so that we eventually investigate. I'll finish reviewing this PR later this weekend. On Fri, Dec 5, 2014 at

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread nkronenfeld
Github user nkronenfeld commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65883687 Should I back out the correction to the mima failure? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread JoshRosen
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65873447 The MiMa failure is surprising, since that class was marked as `private` and therefore shouldn't have been subject to compatibility checks. @ScrapCodes, do you know if

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65870758 [Test build #24195 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24195/consoleFull) for PR 3570 at commit [`b6c2180`](https://gith

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65870763 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24

[GitHub] spark pull request: [SPARK-4772] Clear local copies of accumulator...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3570#issuecomment-65861501 [Test build #24195 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24195/consoleFull) for PR 3570 at commit [`b6c2180`](https://githu