[SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed #31323

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.

Sign up for GitHub

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

Jump to bottom

Closed

LuciferYang wants to merge 14 commits into apache:master from LuciferYang:source-not-closed

Contributor

LuciferYang commented Jan 25, 2021

What changes were proposed in this pull request?

Using a function like .mkString or .getLines directly on a scala.io.Source opened by fromFile, fromURL, fromURI will leak the underlying file handle, this pr use the Utils.tryWithResource method wrap the BufferedSource to ensure these BufferedSource closed.

Why are the changes needed?

Avoid file handle leak.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass the Jenkins or GitHub Action

LuciferYang added 2 commits

January 25, 2021 19:20


          fix not closed

f9a34ba


          rename var

b6bd1f1

SparkQA commented Jan 25, 2021

Test build #134445 has finished for PR 31323 at commit b6bd1f1.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

LuciferYang added 2 commits

January 25, 2021 21:31


          fix format

bc30422


          Merge branch 'upmaster' into source-not-closed

9112d7a

jaceklaskowski suggested changes

View reviewed changes

core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala Outdated Show resolved Hide resolved

core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala Outdated Show resolved Hide resolved

core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala Outdated Show resolved Hide resolved

examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala Outdated Show resolved Hide resolved

...nal/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceOffsetSuite.scala Outdated Show resolved Hide resolved

LuciferYang added 3 commits

January 25, 2021 21:50


          fix some formats

a03934f


          re fix

5b2b96e


          re fix

17fe283

SparkQA commented Jan 25, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39033/

github-actions bot added CORE DSTREAM EXAMPLES SQL STRUCTURED STREAMING WEB UI YARN labels


          re fix

4a51d20

SparkQA commented Jan 25, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39031/

LuciferYang changed the title ~~[SPARK-34224][CORE][TEST] Ensure all resource opened by Source.fromXXX are closed~~ [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][UI][EXAMPLES] Ensure all resource opened by Source.fromXXX are closed

LuciferYang changed the title ~~[SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][UI][EXAMPLES] Ensure all resource opened by Source.fromXXX are closed~~ [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by Source.fromXXX are closed

SparkQA commented Jan 25, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39031/

SparkQA commented Jan 25, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39033/

SparkQA commented Jan 25, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39035/

SparkQA commented Jan 25, 2021

Test build #134447 has finished for PR 31323 at commit bc30422.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA commented Jan 25, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39035/

SparkQA commented Jan 25, 2021

Test build #134449 has finished for PR 31323 at commit 4a51d20.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

jaceklaskowski reviewed

View reviewed changes

core/src/test/scala/org/apache/spark/SparkContextSuite.scala Outdated Show resolved Hide resolved

core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala Show resolved Hide resolved

core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala Show resolved Hide resolved


          fix test failed

40d7f6c

Contributor Author

LuciferYang commented Jan 26, 2021

cc @dongjoon-hyun

HyukjinKwon reviewed

View reviewed changes

external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala Show resolved Hide resolved

HyukjinKwon reviewed

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed

View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed

View reviewed changes

core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed

View reviewed changes

examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala Outdated Show resolved Hide resolved

Member

HyukjinKwon commented Jan 26, 2021

LGTM otherwise

SparkQA commented Jan 26, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39064/

LuciferYang added 3 commits

January 26, 2021 13:28


          revert change of MasterSuite.scala

8e5b51c


          reduce changes

12d4957


          fix HiveExternalCatalogVersionsSuite

9f3c0d5

SparkQA commented Jan 26, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39064/


          fix /DFSReadWriteTest

245d844

LuciferYang commented

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala Show resolved Hide resolved

LuciferYang commented

View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala Show resolved Hide resolved

SparkQA commented Jan 26, 2021

Test build #134478 has finished for PR 31323 at commit 8dfc2b4.

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

SparkQA commented Jan 26, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39069/

SparkQA commented Jan 26, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39069/

HyukjinKwon approved these changes

View reviewed changes

SparkQA commented Jan 26, 2021

Test build #134483 has finished for PR 31323 at commit 245d844.

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

HyukjinKwon pushed a commit that referenced this pull request


          [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure al…

82da778

…l resource opened by `Source.fromXXX` are closed

### What changes were proposed in this pull request?
Using a function like `.mkString` or `.getLines` directly on a `scala.io.Source` opened by `fromFile`, `fromURL`, `fromURI ` will leak the underlying file handle,  this pr use the `Utils.tryWithResource` method wrap the `BufferedSource` to ensure these `BufferedSource` closed.

### Why are the changes needed?
Avoid file handle leak.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass the Jenkins or GitHub Action

Closes #31323 from LuciferYang/source-not-closed.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit 8999e88)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>

HyukjinKwon closed this in

8999e88

Member

HyukjinKwon commented Jan 26, 2021

Merged to master and branch-3.1.

@LuciferYang, do you mind opening a PR for branch-3.0 and branch-2.4 too?

Contributor Author

LuciferYang commented Jan 26, 2021

Merged to master and branch-3.1.

@LuciferYang, do you mind opening a PR for branch-3.0 and branch-2.4 too?

OK, will do it later ~

LuciferYang added a commit to LuciferYang/spark that referenced this pull request


          [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure al…

a9397b7

…l resource opened by `Source.fromXXX` are closed

Using a function like `.mkString` or `.getLines` directly on a `scala.io.Source` opened by `fromFile`, `fromURL`, `fromURI ` will leak the underlying file handle,  this pr use the `Utils.tryWithResource` method wrap the `BufferedSource` to ensure these `BufferedSource` closed.

Avoid file handle leak.

No

Pass the Jenkins or GitHub Action

Closes apache#31323 from LuciferYang/source-not-closed.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>

LuciferYang added a commit to LuciferYang/spark that referenced this pull request


          [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure al…

8606a1d

…l resource opened by `Source.fromXXX` are closed

Using a function like `.mkString` or `.getLines` directly on a `scala.io.Source` opened by `fromFile`, `fromURL`, `fromURI ` will leak the underlying file handle,  this pr use the `Utils.tryWithResource` method wrap the `BufferedSource` to ensure these `BufferedSource` closed.

Avoid file handle leak.

No

Pass the Jenkins or GitHub Action

Closes apache#31323 from LuciferYang/source-not-closed.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>

Contributor Author

LuciferYang commented Jan 26, 2021 •

edited

Loading

Member

dongjoon-hyun commented Jan 26, 2021

Thank you, @LuciferYang and @HyukjinKwon and @jaceklaskowski !

skestle pushed a commit to skestle/spark that referenced this pull request


          [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure al…

40fd840

…l resource opened by `Source.fromXXX` are closed

### What changes were proposed in this pull request?
Using a function like `.mkString` or `.getLines` directly on a `scala.io.Source` opened by `fromFile`, `fromURL`, `fromURI ` will leak the underlying file handle,  this pr use the `Utils.tryWithResource` method wrap the `BufferedSource` to ensure these `BufferedSource` closed.

### Why are the changes needed?
Avoid file handle leak.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass the Jenkins or GitHub Action

Closes apache#31323 from LuciferYang/source-not-closed.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>

LuciferYang deleted the source-not-closed branch

June 6, 2022 03:46

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

Labels

CORE DSTREAM EXAMPLES SQL STRUCTURED STREAMING WEB UI YARN