[GitHub] spark pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-11 Thread srowen
Github user srowen closed the pull request at:

https://github.com/apache/spark/pull/11150


---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182400847
  
**[Test build #51035 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51035/consoleFull)**
 for PR 11150 at commit 
[`d21f7db`](https://github.com/apache/spark/commit/d21f7db9bad878cc0a0e99d7e696fc9631b183ab).
 * 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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182401217
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51035/
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182401215
  
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182521392
  
Thanks for picking this up! Do we still want to keep 
`SparkEnv#closureSerializer`? That's used in quite a few places so it would 
make the patch a little bigger, but I think removing that and its usages would 
make the cleanup most worthwhile.


---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11150#discussion_r52503092
  
--- Diff: core/src/main/scala/org/apache/spark/SparkEnv.scala ---
@@ -277,8 +277,7 @@ object SparkEnv extends Logging {
   "spark.serializer", "org.apache.spark.serializer.JavaSerializer")
 logDebug(s"Using serializer: ${serializer.getClass}")
 
-val closureSerializer = instantiateClassFromConf[Serializer](
-  "spark.closure.serializer", 
"org.apache.spark.serializer.JavaSerializer")
+val closureSerializer = new JavaSerializer(conf)
--- End diff --

alternatively we could make this a field in `SparkEnv` instead of a 
constructor argument, so much of the existing code doesn't need to change.


---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/11150#discussion_r52505440
  
--- Diff: core/src/main/scala/org/apache/spark/SparkEnv.scala ---
@@ -277,8 +277,7 @@ object SparkEnv extends Logging {
   "spark.serializer", "org.apache.spark.serializer.JavaSerializer")
 logDebug(s"Using serializer: ${serializer.getClass}")
 
-val closureSerializer = instantiateClassFromConf[Serializer](
-  "spark.closure.serializer", 
"org.apache.spark.serializer.JavaSerializer")
+val closureSerializer = new JavaSerializer(conf)
--- End diff --

I kept it just because I assume the usages of the closure serializer must 
use the Java serializer, and can't use the generic `spark.serializer` since 
that may be set to Kryo. Sure, there's probably not much point in passing it in 
as an arg, I can change that.


---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182590139
  
LGTM


---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182591522
  
I've merged 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 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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182754703
  
Not sure why github hasn't closed the issue, even though this has been 
merged. @srowen want to close 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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread srowen
GitHub user srowen opened a pull request:

https://github.com/apache/spark/pull/11150

[SPARK-12414] [CORE] Remove closure serializer

Remove spark.closure.serializer option and use JavaSerializer always

CC @andrewor14 @rxin I see there's a discussion in the JIRA but just 
thought I'd offer this for a look at what the change would be.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/srowen/spark SPARK-12414

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/11150.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 #11150


commit d21f7db9bad878cc0a0e99d7e696fc9631b183ab
Author: Sean Owen 
Date:   2016-02-10T12:13:16Z

Remove spark.closure.serializer option and use JavaSerializer always




---
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 pull request: [SPARK-12414] [CORE] Remove closure serializer

2016-02-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11150#issuecomment-182349958
  
**[Test build #51035 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51035/consoleFull)**
 for PR 11150 at commit 
[`d21f7db`](https://github.com/apache/spark/commit/d21f7db9bad878cc0a0e99d7e696fc9631b183ab).


---
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