Skip to content

Comments

[SPARK-43208][SQL][HIVE] IsolatedClassLoader should close barrier class InputStream after reading#40867

Closed
pan3793 wants to merge 1 commit intoapache:masterfrom
pan3793:SPARK-43208
Closed

[SPARK-43208][SQL][HIVE] IsolatedClassLoader should close barrier class InputStream after reading#40867
pan3793 wants to merge 1 commit intoapache:masterfrom
pan3793:SPARK-43208

Conversation

@pan3793
Copy link
Member

@pan3793 pan3793 commented Apr 20, 2023

What changes were proposed in this pull request?

As title, close the barrier class InputStream after reading in IsolatedClassLoader

Why are the changes needed?

IOUtils.toByteArray(inputStream) is not responsible to close the inputStream, the caller should do closing instead.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

@github-actions github-actions bot added the SQL label Apr 20, 2023
@LuciferYang
Copy link
Contributor

Good catch

if (loaded == null) doLoadClass(name, resolve) else loaded
}
def doLoadClass(name: String, resolve: Boolean): Class[_] = {
val classFileName = name.replaceAll("\\.", "/") + ".class"
Copy link
Member Author

Choose a reason for hiding this comment

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

it's exactly same as classToPath(name)

@pan3793
Copy link
Member Author

pan3793 commented Apr 20, 2023

cc @HyukjinKwon @sunchao

@sunchao sunchao changed the title [SPARK-43208][HIVE] IsolatedClassLoader should close barrier class InputStream after reading [SPARK-43208][SQL][HIVE] IsolatedClassLoader should close barrier class InputStream after reading Apr 20, 2023
@sunchao sunchao closed this in 01dc1cb Apr 20, 2023
@sunchao
Copy link
Member

sunchao commented Apr 20, 2023

@pan3793 good catch, have you seen any issue caused by this?

@sunchao
Copy link
Member

sunchao commented Apr 20, 2023

Merged to master, thanks!

@pan3793
Copy link
Member Author

pan3793 commented Apr 20, 2023

@pan3793 good catch, have you seen any issue caused by this?

Not exactly. Since the current Hive 2.3.9 can talk w/ HMS 2.1+ smoothly (plus HIVE-25500, the HMS versions expand to 1.2+), it's rare to use a custom Hive client version in our production cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants