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-18646][REPL] Set parent classloader as null for ExecutorClassLoader #18614

Closed
wants to merge 8 commits into from

Conversation

jiangxb1987
Copy link
Contributor

@jiangxb1987 jiangxb1987 commented Jul 12, 2017

What changes were proposed in this pull request?

ClassLoader will preferentially load class from parent. Only when parent is null or the load failed, that it will call the overridden findClass function. To avoid the potential issue caused by loading class using inappropriate class loader, we should set the parent of ClassLoader to null, so that we can fully control which class loader is used.

This is take over of #17074, the primary author of this PR is @taroplus .

Should close #17074 after this PR get merged.

How was this patch tested?

Add test case in ExecutorClassLoaderSuite.

@cloud-fan
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Jul 12, 2017

Test build #79564 has finished for PR 18614 at commit d26f89f.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM, too.

asfgit pushed a commit that referenced this pull request Jul 13, 2017
…oader

## What changes were proposed in this pull request?

`ClassLoader` will preferentially load class from `parent`. Only when `parent` is null or the load failed, that it will call the overridden `findClass` function. To avoid the potential issue caused by loading class using inappropriate class loader, we should set the `parent` of `ClassLoader` to null, so that we can fully control which class loader is used.

This is take over of #17074,  the primary author of this PR is taroplus .

Should close #17074 after this PR get merged.

## How was this patch tested?

Add test case in `ExecutorClassLoaderSuite`.

Author: Kohki Nishio <taroplus@me.com>
Author: Xingbo Jiang <xingbo.jiang@databricks.com>

Closes #18614 from jiangxb1987/executor_classloader.

(cherry picked from commit e08d06b)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

cloud-fan commented Jul 13, 2017

thanks, merging to master!

@asfgit asfgit closed this in e08d06b Jul 13, 2017
MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
…oader

## What changes were proposed in this pull request?

`ClassLoader` will preferentially load class from `parent`. Only when `parent` is null or the load failed, that it will call the overridden `findClass` function. To avoid the potential issue caused by loading class using inappropriate class loader, we should set the `parent` of `ClassLoader` to null, so that we can fully control which class loader is used.

This is take over of apache#17074,  the primary author of this PR is taroplus .

Should close apache#17074 after this PR get merged.

## How was this patch tested?

Add test case in `ExecutorClassLoaderSuite`.

Author: Kohki Nishio <taroplus@me.com>
Author: Xingbo Jiang <xingbo.jiang@databricks.com>

Closes apache#18614 from jiangxb1987/executor_classloader.

(cherry picked from commit e08d06b)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
rdblue pushed a commit to rdblue/spark that referenced this pull request Apr 3, 2019
…oader

## What changes were proposed in this pull request?

`ClassLoader` will preferentially load class from `parent`. Only when `parent` is null or the load failed, that it will call the overridden `findClass` function. To avoid the potential issue caused by loading class using inappropriate class loader, we should set the `parent` of `ClassLoader` to null, so that we can fully control which class loader is used.

This is take over of apache#17074,  the primary author of this PR is taroplus .

Should close apache#17074 after this PR get merged.

## How was this patch tested?

Add test case in `ExecutorClassLoaderSuite`.

Author: Kohki Nishio <taroplus@me.com>
Author: Xingbo Jiang <xingbo.jiang@databricks.com>

Closes apache#18614 from jiangxb1987/executor_classloader.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants