[GitHub] spark pull request: [SPARK-5423][Core] Cleanup resources in DiskMa...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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