[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-20 Thread zuotingbing
Github user zuotingbing commented on the issue: https://github.com/apache/spark/pull/19277 I am not sure. If we move this line `val in = new BufferedInputStream(fs.open(log))` into try~catch, we should define `var in: BufferedInputStream = null` before, and use `catch { case e: Except

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-20 Thread jerryshao
Github user jerryshao commented on the issue: https://github.com/apache/spark/pull/19277 Strictly saying, this line `new BufferedInputStream(fs.open(log))` will also throw exception, shouldn't you try-catch it? ---

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-20 Thread zuotingbing
Github user zuotingbing commented on the issue: https://github.com/apache/spark/pull/19277 @jerryshao if this line `new BufferedInputStream(fs.open(log))` throws exception, it mean we do not need to close the object of BufferedInputStream because it new failed. --- --

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-20 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/19277 @zuotingbing no, `fs.open` could succeed, but `new BufferedInputStream` could fail, leaving the underlying stream open. In practice, it can't actually fail because this constructor does no I/O or ope

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-21 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/19277 What other instances of try-catch might need improvement like this? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-22 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19277 **[Test build #3929 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3929/testReport)** for PR 19277 at commit [`e3f8e0d`](https://github.com/apache/spark/commit/e

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-22 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19277 **[Test build #3929 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3929/testReport)** for PR 19277 at commit [`e3f8e0d`](https://github.com/apache/spark/commit/

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-22 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19277 **[Test build #3932 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3932/testReport)** for PR 19277 at commit [`2e5f21a`](https://github.com/apache/spark/commit/2

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19277 **[Test build #3933 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3933/testReport)** for PR 19277 at commit [`2e5f21a`](https://github.com/apache/spark/commit/2

[GitHub] spark issue #19277: [SPARK-22058][CORE]the BufferedInputStream will not be c...

2017-09-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19277 **[Test build #3933 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3933/testReport)** for PR 19277 at commit [`2e5f21a`](https://github.com/apache/spark/commit/