[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-03-03 Thread zsxwing
Github user zsxwing commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-77075510
  
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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-03-03 Thread nishkamravi2
Github user nishkamravi2 commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-77041199
  
Sounds good, thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-03-03 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-77033874
  
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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-03-03 Thread nishkamravi2
Github user nishkamravi2 commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-77022278
  
@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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-02-19 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/4219


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-02-19 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-75108739
  
Ok, LGTM I'm merging this into master and 1.3 thanks for fixing this 
@zsxwing 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-01-27 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-71615700
  
  [Test build #26153 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26153/consoleFull)
 for   PR 4219 at commit 
[`d4b2ca6`](https://github.com/apache/spark/commit/d4b2ca69b3bc2d729f5d44750ab6b81de6e77644).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-01-27 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-71615711
  
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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-01-27 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/4219#issuecomment-71607739
  
  [Test build #26153 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26153/consoleFull)
 for   PR 4219 at commit 
[`d4b2ca6`](https://github.com/apache/spark/commit/d4b2ca69b3bc2d729f5d44750ab6b81de6e77644).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...

2015-01-27 Thread zsxwing
GitHub user zsxwing opened a pull request:

https://github.com/apache/spark/pull/4219

[SPARK-5423][Core] Cleanup resources in DiskMapIterator.finalize to ensure 
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.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zsxwing/spark SPARK-5423

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/4219.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4219


commit d4b2ca69b3bc2d729f5d44750ab6b81de6e77644
Author: zsxwing 
Date:   2015-01-27T08:16:13Z

Cleanup resources in DiskMapIterator.finalize to ensure deleting the temp 
file




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org