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.

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

[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file #4219

Closed
wants to merge 1 commit into from
Closed

[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file #4219

wants to merge 1 commit into from
+43 −9

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Jan 27, 2015

This PR adds a finalize method in DiskMapIterator to clean up the resources even if some exception happens during processing data.

@SparkQA
Copy link

SparkQA commented Jan 27, 2015

Test build #26153 has started for PR 4219 at commit d4b2ca6.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 27, 2015

Test build #26153 has finished for PR 4219 at commit d4b2ca6.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26153/
Test PASSed.

@andrewor14
Copy link
Contributor

Ok, LGTM I'm merging this into master, 1.3, 1.2, and 1.1. thanks for fixing this @zsxwing

asfgit pushed a commit that referenced this pull request Feb 19, 2015
@zsxwing
[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to e…
25fae8e 
…nsure deleting the temp file

This PR adds a `finalize` method in DiskMapIterator to clean up the resources even if some exception happens during processing data.

Author: zsxwing <zsxwing@gmail.com>

Closes #4219 from zsxwing/SPARK-5423 and squashes the following commits:

d4b2ca6 [zsxwing] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file
(cherry picked from commit 90095bf)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-36-14.us-west-2.compute.internal>
asfgit pushed a commit that referenced this pull request Feb 19, 2015
@zsxwing
[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to e…
61bde00 
…nsure deleting the temp file

This PR adds a `finalize` method in DiskMapIterator to clean up the resources even if some exception happens during processing data.

Author: zsxwing <zsxwing@gmail.com>

Closes #4219 from zsxwing/SPARK-5423 and squashes the following commits:

d4b2ca6 [zsxwing] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file
(cherry picked from commit 90095bf)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-36-14.us-west-2.compute.internal>
@asfgit asfgit closed this in 90095bf Feb 19, 2015
asfgit pushed a commit that referenced this pull request Feb 19, 2015
@zsxwing
[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to e…
36f3c49 
…nsure deleting the temp file

This PR adds a `finalize` method in DiskMapIterator to clean up the resources even if some exception happens during processing data.

Author: zsxwing <zsxwing@gmail.com>

Closes #4219 from zsxwing/SPARK-5423 and squashes the following commits:

d4b2ca6 [zsxwing] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file
(cherry picked from commit 90095bf)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-36-14.us-west-2.compute.internal>
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Feb 24, 2015
@zsxwing @markhamstra
[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to e…
f295da3 
…nsure deleting the temp file

This PR adds a `finalize` method in DiskMapIterator to clean up the resources even if some exception happens during processing data.

Author: zsxwing <zsxwing@gmail.com>

Closes apache#4219 from zsxwing/SPARK-5423 and squashes the following commits:

d4b2ca6 [zsxwing] Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp file
(cherry picked from commit 90095bf)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-36-14.us-west-2.compute.internal>
@zsxwing zsxwing deleted the SPARK-5423 branch February 25, 2015 04:16
@nishkamravi2
Copy link
Contributor

@zsxwing @andrewor14 I'm noticing a significant performance regression with this commit (SPARK-6142). Commenting out finalize recovers performance (as expected). Finalize is generally considered unhealthy for production code (can create indefinite performance issues and doesn't guarantee cleanup). Should we consider reverting this fix for 1.3 and contemplating an alternate one?

@andrewor14
Copy link
Contributor

Thanks for reporting this. I will just revert this one altogether and consider a different alternative after the release. This bug has been an issue since 1.0 and is not so critical that we have to rush in an alternative fix for the release. In the future we may want to consider an alternative that uses weak references instead.

@nishkamravi2
Copy link
Contributor

Sounds good, thanks.

@zsxwing
Copy link
Member Author

zsxwing commented Mar 4, 2015

How many failure tasks in your test? If no failure task, looks weird. In the happy path, the finalize method should only check closed and exit. @nishkamravi2 do you have any number about the significant performance regression?

dongjoon-hyun pushed a commit that referenced this pull request Dec 27, 2023
@LuciferYang @dongjoon-hyun
[SPARK-46508][BUILD] Upgrade Jackson to 2.16.1
5a5d3c7 
### What changes were proposed in this pull request?
This pr aims to upgrade jackson from 2.16.0 to 2.16.1

### Why are the changes needed?
The new version bring some fix:

- [#4200](FasterXML/jackson-databind#4200): JsonSetter(contentNulls = FAIL) is ignored in delegating JsonCreator argument
- [#4216](FasterXML/jackson-databind#4216): Primitive array deserializer not being captured by DeserializerModifier
- [#4219](FasterXML/jackson-databind#4219): JsonNode.findValues() and findParents() missing expected values in 2.16.0

The full release notes as follows:

- https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.16.1

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

### How was this patch tested?
Pass Github Actions

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #44494 from LuciferYang/SPARK-46508.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@zsxwing @SparkQA @AmplabJenkins @andrewor14 @nishkamravi2