Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-21928][CORE] Set classloader on SerializerManager's private kryo #19313

Closed
wants to merge 1 commit into from

Conversation

squito
Copy link
Contributor

@squito squito commented Sep 21, 2017

What changes were proposed in this pull request?

We have to make sure that SerializerManager's private instance of
kryo also uses the right classloader, regardless of the current thread
classloader. In particular, this fixes serde during remote cache
fetches, as those occur in netty threads.

How was this patch tested?

Manual tests & existing suite via jenkins. I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally. I manually verified the warning message is no longer present.

(cherry picked from commit b75bd17)

We have to make sure that SerializerManager's private instance of
kryo also uses the right classloader, regardless of the current thread
classloader.  In particular, this fixes serde during remote cache
fetches, as those occur in netty threads.

Manual tests & existing suite via jenkins.  I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally.  I manually verified the warning message is no longer present.

Author: Imran Rashid <irashid@cloudera.com>

Closes apache#19280 from squito/SPARK-21928_ser_classloader.

(cherry picked from commit b75bd17)
@SparkQA
Copy link

SparkQA commented Sep 21, 2017

Test build #82049 has finished for PR 19313 at commit 3a6b4ef.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor

vanzin commented Sep 21, 2017

Merging to 2.1.

@vanzin
Copy link
Contributor

vanzin commented Sep 21, 2017

(Please close the PR manually.)

asfgit pushed a commit that referenced this pull request Sep 21, 2017
## What changes were proposed in this pull request?

We have to make sure that SerializerManager's private instance of
kryo also uses the right classloader, regardless of the current thread
classloader.  In particular, this fixes serde during remote cache
fetches, as those occur in netty threads.

## How was this patch tested?

Manual tests & existing suite via jenkins.  I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally.  I manually verified the warning message is no longer present.

(cherry picked from commit b75bd17)

Author: Imran Rashid <irashid@cloudera.com>

Closes #19313 from squito/SPARK-21928_2.1.
@squito
Copy link
Contributor Author

squito commented Sep 21, 2017

thanks @vanzin

@squito squito closed this Sep 21, 2017
@squito squito deleted the SPARK-21928_2.1 branch September 25, 2017 20:57
jzhuge pushed a commit to jzhuge/spark that referenced this pull request Aug 20, 2018
We have to make sure that SerializerManager's private instance of
kryo also uses the right classloader, regardless of the current thread
classloader.  In particular, this fixes serde during remote cache
fetches, as those occur in netty threads.

Manual tests & existing suite via jenkins.  I haven't been able to reproduce this is in a unit test, because when a remote RDD partition can be fetched, there is a warning message and then the partition is just recomputed locally.  I manually verified the warning message is no longer present.

(cherry picked from commit b75bd17)

Author: Imran Rashid <irashid@cloudera.com>

Closes apache#19313 from squito/SPARK-21928_2.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants