Uh oh!
There was an error while loading. Please reload this page.
[CORE][MINOR]Closes stream and releases any system resources associated with this stream - #18522
[CORE][MINOR]Closes stream and releases any system resources associated with this stream#1852210110346 wants to merge 1 commit into
Conversation
| } { | ||
| closeFile() | ||
| try { | ||
| inputStream.close() |
There was a problem hiding this comment.
AFAIK this inputStream is passed from outside, like process.getInputStream in ExecutorRunner, so I don't think we should close the inputStream here.
There was a problem hiding this comment.
Yes,you are right.
But this function is only used in ExecutorRunner, also if an exception occurs within this function,this will ensure the inputStream is closed
There was a problem hiding this comment.
The another reason is that this function runs in another thread
There was a problem hiding this comment.
It shouldn't be closed here, because the lifecycle of this stream is outside of this class. In practice, the streams used with this class are closed on error anyway.
There was a problem hiding this comment.
I referred to DriverRunner:
DriverRunner-->runDriver-->redirectStream-->copyStream, the inputStream is closed in copyStream.
SparkQA
commented
Jul 4, 2017
Test build #79120 has finished for PR 18522 at commit
|
| test("resolveURIs with multiple paths") { | ||
| def assertResolves(before: String, after: String): Unit = { | ||
| assume(before.split(",").length > 1) | ||
| assume(before.split(",").length >= 1) |
There was a problem hiding this comment.
When running UtilsSuite, the following warning occurred:
Test Canceled: 1 was not greater than 1
org.scalatest.exceptions.TestCanceledException: 1 was not greater than 1
at org.scalatest.Assertions$class.newTestCanceledException(Assertions.scala:511)
at org.scalatest.FunSuite.newTestCanceledException(FunSuite.scala:1555)
at org.scalatest.Assertions$AssertionsHelper.macroAssume(Assertions.scala:481)
at org.apache.spark.util.UtilsSuite$$anonfun$19.assertResolves$2(UtilsSuite.scala:491)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply$mcV$sp(UtilsSuite.scala:512)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply(UtilsSuite.scala:489)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply(UtilsSuite.scala:489)
10110346
commented
Jul 31, 2017
|
srowen
commented
Jul 31, 2017
I suspect that this doesn't hurt, because at the point you stop copying input to a file, you are done with the input, and I don't think there is any reason that the caller would ever continue reading it elsewhere. That said, the process is already terminated correctly in this case, which closes the streams too. I'm neutral on it unless this is theoretically solving a problem. |
10110346
commented
Aug 20, 2017
Thanks @srowen |
srowen
commented
Aug 20, 2017
I'm not sure that this solves a problem, and has some changes that look unrelated |
Closesapache#18522Closesapache#17722Closesapache#18879Closesapache#18891Closesapache#18806Closesapache#18948Closesapache#18949Closesapache#19070Closesapache#19039Closesapache#19142Closesapache#18515Closesapache#19154Closesapache#19162Closesapache#19187
Closesapache#18522Closesapache#17722Closesapache#18879Closesapache#18891Closesapache#18806Closesapache#18948Closesapache#18949Closesapache#19070Closesapache#19039Closesapache#19142Closesapache#18515Closesapache#19154Closesapache#19162Closesapache#19187Closesapache#19091 Author: Sean Owen <sowen@cloudera.com> Closesapache#19203 from srowen/CloseStalePRs3.
What changes were proposed in this pull request?
Closes inputstream or outputstream and releases any system resources associated
with the stream.
How was this patch tested?
unit test