You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by darabos <gi...@git.apache.org> on 2014/03/19 20:02:19 UTC
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
GitHub user darabos opened a pull request:
https://github.com/apache/spark/pull/181
Use the Executor's ClassLoader in sc.objectFile().
This makes it possible to read classes from the object file which were specified in the user-provided jars. (By default ObjectInputStream uses latestUserDefinedLoader, which may or may not be the right one.)
I created this because I ran into the following problem. I have x:RDD[X] with X being defined in the jar that I provide to SparkContext. I save it with x.saveAsObjectFile("x"). I try to load it with sc.objectFile[X]("x"). It fails with ClassNotFoundException.
After a good while of debugging I figured out that Utils.deserialize() most likely uses the ClassLoader of Utils. This is the bootstrap ClassLoader, so it is not aware of the dynamically added jars. This patch fixes the issue.
A more robust fix would be to always default to Thread.currentThread.getContextClassLoader. This would prevent this problem from biting anyone in the future. It would be a bit harder to test though. On the topic of testing, if you'd like to see tests for this, I will need some hand-holding. Thanks!
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/darabos/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/181.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 #181
----
commit 1b5df2cb5a69e357b1eb0b19cc05f4b1aedb5701
Author: Daniel Darabos <da...@gmail.com>
Date: 2014-03-19T18:52:12Z
Use the Executor's ClassLoader in sc.objectFile(). This makes it possible to read classes from the object file which were specified in the user-provided jars. (By default ObjectInputStream uses latestUserDefinedLoader, which may or may not be the right one.)
----
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-47647756
I was so slow, Bogdan has already fixed this in #821. Anyway, here's the belated test. It's probably still useful to avoid regressions. I tested the test by reverting Bogdan's change, and the test then fails with `ClassNotFoundException: FileSuiteObjectFileTest`. Either both the fix and the test are correct, or they are both bugged :).
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-47072331
@darabos do you mind picking this up now the test util was merged?
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-48704142
Cool - thanks for contributing the test! Jenkins, test this please. LGTM pending tests.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107315
Merged build finished.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-48804476
@darabos yeah you're just seeing a bug in some new code we wrote to post information about pull requests to github. I think we don't properly escape quotes in this code. Anyways, no problem, tests have passed so I can merge 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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-47074387
Sorry for leaving this hanging. I'll take a look at the test.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-48704291
QA tests have started for PR 181. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16563/consoleFull
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38881871
Can one of the admins verify this patch?
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-40141621
Hey forgot to ping this, but I included that test utility a while ago in a pull request that we merged.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38114317
I'll fix the style error and look at writing a test. Thanks for the pointers!
What do you think about using Thread.currentThread.getContextClassLoader in deserialize by default? Since deserialize is not part of the public interface I think this may be safe and useful to do.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107183
Merged build started.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107653
@darabos unfortunately testing this is kind of tricky because we need to create a .jar with a class that is not normally on the classpath. There's some TestUtils code to create this in https://github.com/apache/spark/pull/119/files, which compiles a new class. Maybe we can get for that to get merged. We can also try to create a dummy JAR through our own build process and find it, but it will be kind of brittle.
Once we have such a JAR, I'd add a test to FileServerSuite that tries to use those classes in tasks. This is already used to test that addJar ships the JAR over.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/181
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-48796852
I think Jenkins means to say it's all good.
~~~
Attempting to post to Github:
{"body": "QA results for PR 181:<br>
- This patch PASSES unit tests.<br>
- This patch merges cleanly<br>
- This patch adds the following public classes (experimental):<br>
- "public class " className + " { @Override public String toString() { " +<br>
"public class " + className + " implements java.io.Serializable {" +<br>
<br>
For more information see test ouptut:<br>
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16563/consoleFull"}
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 553 100 99 100 454 288 1322 --:--:-- --:--:-- --:--:-- 1351
HTTP/1.1 400 Bad Request
Server: GitHub.com
Date: Fri, 11 Jul 2014 09:38:58 GMT
Content-Type: application/json; charset=utf-8
Status: 400 Bad Request
~~~
(Extra linebreaks by me.) Not sure what went wrong with the request. Perhaps the quotes in the mistakenly identified class names got encoded wrong. But it looks like I probably didn't break anything :).
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107182
Merged build triggered.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107704
BTW the "test failure" is actually a style error if you click it -- you have a line longer than 100 characters.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107316
One or more automated tests failed
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13274/
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38107071
Jenkins, test this please
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-40141612
Hey forgot to ping this, but I included that test utility a while ago in a pull request that we merged.
---
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.
---
[GitHub] spark issue #181: Use the Executor's ClassLoader in sc.objectFile().
Posted by ch0ice <gi...@git.apache.org>.
Github user ch0ice commented on the issue:
https://github.com/apache/spark/pull/181
This problem arose again for me, and I reproduced it when I converted byte into protoBuf after redis checked the data.
The following code in the deserialization (Utils deserialize (value, Utils getContextOrSparkClassLoader ())) when there is an error
Map<byte[],byte[]> hResult = jedisUtil.hgetAll((REDIS_LASTPOINT_PREFIX + serviceId));
hResult.forEach((key,value)->{
LocationPackage.LocationMessage lastLocationPackage = Utils.deserialize(value,Utils.getContextOrSparkClassLoader());
if(lastLocationPackage.getLocation().getLocationTime() >= sTime && lastLocationPackage.getLocation().getLocationTime() <= eTime){
String gpsRedisKey = REDIS_GPSLIST_PREFIX + serviceId + "_" + lastLocationPackage.getLocation().getTerminalId();
redisKeys.add(gpsRedisKey);
}
});
ERROR:
java.lang.RuntimeException: Unable to find proto buffer class
at com.google.protobuf.GeneratedMessageLite$SerializedForm.readResolve(GeneratedMessageLite.java:775)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at java.io.ObjectStreamClass.invokeReadResolve(ObjectStreamClass.java:1104)
at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1810)
at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351)
at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371)
at org.apache.spark.util.Utils$.deserialize(Utils.scala:161)
at org.apache.spark.util.Utils.deserialize(Utils.scala)
at com.location.platform.analysis.util.AnalysisUtil.lambda$getRedisData$0(AnalysisUtil.java:310)
at com.location.platform.analysis.util.AnalysisUtil$$Lambda$17/489334328.accept(Unknown Source)
at java.util.Map.forEach(Map.java:630)
at com.location.platform.analysis.util.AnalysisUtil.getRedisData(AnalysisUtil.java:307)
at com.location.platform.analysis.util.AnalysisUtil.getMongoAndRedisDataset(AnalysisUtil.java:372)
at com.location.platform.analysis.bi.TodayOdometerAnalysis.lambda$main$0(TodayOdometerAnalysis.java:96)
at com.location.platform.analysis.bi.TodayOdometerAnalysis$$Lambda$1/1413730361.accept(Unknown Source)
at java.util.ArrayList.forEach(ArrayList.java:1249)
at com.location.platform.analysis.bi.TodayOdometerAnalysis.main(TodayOdometerAnalysis.java:76)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.apache.spark.deploy.SparkSubmit$.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:775)
at org.apache.spark.deploy.SparkSubmit$.doRunMain$1(SparkSubmit.scala:180)
at org.apache.spark.deploy.SparkSubmit$.submit(SparkSubmit.scala:205)
at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:119)
at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
Caused by: java.lang.ClassNotFoundException: com.location.platform.common.protobuf.LocationPackage$LocationMessage
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:264)
at com.google.protobuf.GeneratedMessageLite$SerializedForm.readResolve(GeneratedMessageLite.java:768)
... 28 more
Only if there is a problem with the cluster, local mode is fine.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38909539
Sorry, the delay is my fault. I was too busy to get around to the test so far, but I still intend to do it. At least I've _read_ some Spark tests :).
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by darabos <gi...@git.apache.org>.
Github user darabos commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38928010
I couldn't figure out how to get a separate jar built for use in this test. (I'm new to Java/Scala build systems.) Anyway you say it would be brittle. I'll just wait for TestUtils from https://github.com/apache/spark/pull/119. It looks like a generally handy tool for testing jar-related behavior.
---
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.
---
[GitHub] spark pull request: Use the Executor's ClassLoader in sc.objectFil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/181#issuecomment-38093151
Can one of the admins verify this patch?
---
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.
---