[GitHub] spark pull request #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r86767759
  
--- Diff: 
streaming/src/test/scala/org/apache/spark/streaming/MapWithStateSuite.scala ---
@@ -38,19 +38,16 @@ class MapWithStateSuite extends SparkFunSuite
   protected val batchDuration = Seconds(1)
 
   before {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-checkpointDir = Utils.createTempDir("checkpoint")
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   after {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-if (checkpointDir != null) {
-  Utils.deleteRecursively(checkpointDir)
-}
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   override def beforeAll(): Unit = {
 super.beforeAll()
+checkpointDir = Utils.createTempDir("checkpoint")
--- End diff --

I believe this change is not valid. I will get rid of this. Thank you for 
pointing this out.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r86767539
  
--- Diff: 
streaming/src/test/scala/org/apache/spark/streaming/MapWithStateSuite.scala ---
@@ -38,19 +38,16 @@ class MapWithStateSuite extends SparkFunSuite
   protected val batchDuration = Seconds(1)
 
   before {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-checkpointDir = Utils.createTempDir("checkpoint")
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   after {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-if (checkpointDir != null) {
-  Utils.deleteRecursively(checkpointDir)
-}
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   override def beforeAll(): Unit = {
 super.beforeAll()
+checkpointDir = Utils.createTempDir("checkpoint")
--- End diff --

Actually, it seems the original codes are already fine in here.. I took a 
look and ran several tests and it seems now I may understand why @taoli91 tried 
to fix the codes like this.

It seems somehow the state in `ReceiverTracker` went wrong on Windows and 
ended up without closing `checkpointDir`. It seems the original test was 
failed[1] due to the issue in `ReceiverTracker`. So, I think he tried to only 
create/delete the folder once[2] and ensure closing in `ReceiverTracker`.

I tested this with manually stopping  `ReceivedBlockTracker` regardless of 
the state (the original proposal) and it seems fine without the changes in 
here, `MapWithStateSuite.scala`[3]. Of course,  it is fine with this change[4]. 



[1]https://ci.appveyor.com/project/spark-test/spark/build/56-F88EDDAF-E576-4787-9530-A4185FC46B1E

[2]https://ci.appveyor.com/project/spark-test/spark/build/57-test-MapWithStateSuite

[3]https://ci.appveyor.com/project/spark-test/spark/build/58-test-MapWithStateSuite

[4]https://ci.appveyor.com/project/spark-test/spark/build/59-test-MapWithStateSuite


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r86687384
  
--- Diff: 
streaming/src/test/scala/org/apache/spark/streaming/MapWithStateSuite.scala ---
@@ -38,19 +38,16 @@ class MapWithStateSuite extends SparkFunSuite
   protected val batchDuration = Seconds(1)
 
   before {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-checkpointDir = Utils.createTempDir("checkpoint")
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   after {
-StreamingContext.getActive().foreach { _.stop(stopSparkContext = 
false) }
-if (checkpointDir != null) {
-  Utils.deleteRecursively(checkpointDir)
-}
+StreamingContext.getActive().foreach(_.stop(stopSparkContext = false))
   }
 
   override def beforeAll(): Unit = {
 super.beforeAll()
+checkpointDir = Utils.createTempDir("checkpoint")
--- End diff --

This might happen to be OK, but now the dir is not deleted between tests. 
Is that going to be OK?


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon closed the pull request at:

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


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

GitHub user HyukjinKwon reopened a pull request:

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

[SPARK-14914][CORE] Fix Resource not closed after using, mostly for unit 
tests

## What changes were proposed in this pull request?

Close `FileStreams`, `ZipFiles` etc to release the resources after using. 
Not closing the resources will cause IO Exception to be raised while deleting 
temp files.
## How was this patch tested?

Existing tests


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

$ git pull https://github.com/HyukjinKwon/spark SPARK-14914-1

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

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


commit ceb1681620b83e46425d20f20ff5a228c0a8d75b
Author: U-FAREAST\tl 
Date:   2016-04-25T03:37:06Z

mvn.cmd for windows

Helping script for windows to download dependency and start zinc to
support incremental building on windows.

commit 98ee4ab6a96b3d8ad8acd8dbeb6dedc10523546e
Author: U-FAREAST\tl 
Date:   2016-04-25T06:50:59Z

Fix file not closed on FileSuite

commit ed52ec72fee774e9b8a1bec3183ae4594d758b9f
Author: U-FAREAST\tl 
Date:   2016-04-25T07:45:39Z

Fix another file closing

commit 448508f467c0401f5461d79dadc1454b03210368
Author: Tao LI 
Date:   2016-04-25T07:57:42Z

close log data.

commit 34078a3facde46f09430baf1a1a5976b1c2d2869
Author: Tao LI 
Date:   2016-04-25T08:01:55Z

Close the class loader

commit efb7227518e9fdb8c1a35ae2adb3971c9cfc1ac2
Author: Tao LI 
Date:   2016-04-25T08:20:48Z

Another file not closed.

commit a06bffc02e4d2cd7e723e73102c160c1c57f0915
Author: Tao LI 
Date:   2016-04-25T08:29:16Z

Stop to release resources.

commit 45262dcc58073a99417b9d1a6c0e24c393716c8f
Author: U-FAREAST\tl 
Date:   2016-04-25T08:34:45Z

More closing problem

commit b3c0c96fb4cbe19344cc220e234c98644aa0efcd
Author: U-FAREAST\tl 
Date:   2016-04-25T13:25:42Z

Fix the zip file and jar file in RPackageUtilsSuite

commit a176adbadfa68cb0819a1e958129c4d96b42b42c
Author: U-FAREAST\tl 
Date:   2016-04-26T05:35:35Z

Stop ssc in MasterFailureTest

commit 35aacd29c4667550a4f870ee521ed185c5f9800c
Author: U-FAREAST\tl 
Date:   2016-04-26T08:09:04Z

Remove accidentally added files

commit 9f50128da0660fed97d64b8a5e0d63285dbf93d5
Author: U-FAREAST\tl 
Date:   2016-05-03T06:11:12Z

Code cleanup with respect to comments

commit 55b360e276968eecea970267b0fa438b56e5e703
Author: U-FAREAST\tl 
Date:   2016-05-05T03:51:50Z

Style fixes

commit 91f82b5fac48afefeceadd764ff0e7b61944d875
Author: U-FAREAST\tl 
Date:   2016-05-05T07:29:32Z

Minor code cleanup

commit f3713d1ff59e5bd45a8a207eaf36ab8e6c285812
Author: hyukjinkwon 
Date:   2016-10-25T05:44:15Z

ex -> e and indentation

commit 863ea7f66d4919a3e2c8ee6e3ca575a80a3115dc
Author: hyukjinkwon 
Date:   2016-10-27T15:55:39Z

Use Utils.tryWithSafeFinally where possible.

commit 3949dbeda632e215c64dcc089365bdc7334dacaf
Author: hyukjinkwon 
Date:   2016-10-29T02:02:43Z

close loader later

commit 49cb4e7f259ba0a236b0a977e69719cfc165c265
Author: hyukjinkwon 
Date:   2016-10-30T12:07:32Z

Initialize receivedBlockTracker in start()

commit 15215722dfe2de0785d38cce5713f33fac5e4b03
Author: hyukjinkwon 
Date:   2016-11-01T15:43:39Z

Require nulls for eventpoint and the tracker in start and remove other 
checks in methods




---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85934066
  
--- Diff: 
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceiverTracker.scala
 ---
@@ -154,6 +147,17 @@ class ReceiverTracker(ssc: StreamingContext, 
skipReceiverLaunch: Boolean = false
   throw new SparkException("ReceiverTracker already started")
 }
 
+if (receivedBlockTracker == null) {
--- End diff --

Thank you so much for detailed explanation. Will try to follow this.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85663718
  
--- Diff: 
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceiverTracker.scala
 ---
@@ -202,18 +206,21 @@ class ReceiverTracker(ssc: StreamingContext, 
skipReceiverLaunch: Boolean = false
 
   /** Allocate all unallocated blocks to the given batch. */
   def allocateBlocksToBatch(batchTime: Time): Unit = {
+require(receivedBlockTracker != null, "ReceiverTracker should be 
started first.")
--- End diff --

Per comments above, I don't know if you need to assert about the state 
here. I suppose you'll already get an error if this isn't true, but, this would 
indicate something else already went wrong.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85663898
  
--- Diff: 
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceiverTracker.scala
 ---
@@ -154,6 +147,17 @@ class ReceiverTracker(ssc: StreamingContext, 
skipReceiverLaunch: Boolean = false
   throw new SparkException("ReceiverTracker already started")
 }
 
+if (receivedBlockTracker == null) {
--- End diff --

It looks like this is meant to be restartable, in that it will allow a 
stopped tracker to call `start()` again. If that's the case, then 
`receivedBlockTracker` needs to be nulled in `stop()`, and then this check 
isn't needed I think because it should really be null. In fact if you like you 
can require that it's null, and that endpoint is null, in `start()`?

But then ... I think this method needs to throw an exception also if 
`isTrackerStopping` at the start. It won't observe the intermediate state 
because of syncrhonization, but it could be left in this state if stopping 
failed.

I am wary of going too far down a path of fixing or changing the corner 
cases here. I do agree that there's a legitimate mismatch between where the 
tracker is initialized and stopped, and I think the changes you made and that 
possibly the ones I mention here together improve the situation without 
complicating it further.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85630990
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -209,7 +209,8 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 
   assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
   assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
-} finally {
+} {
+  loader.close()
   Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End diff --

Sure, makes sense.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[mridulm]

Github user mridulm commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85436193
  
--- Diff: 
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceiverTracker.scala
 ---
@@ -194,10 +194,13 @@ class ReceiverTracker(ssc: StreamingContext, 
skipReceiverLaunch: Boolean = false
   // Finally, stop the endpoint
   ssc.env.rpcEnv.stop(endpoint)
   endpoint = null
-  receivedBlockTracker.stop()
   logInfo("ReceiverTracker stopped")
   trackerState = Stopped
 }
+
+// note that the output writer is created at construction time, we 
have to close
+// them even if it hasn't been started.
+receivedBlockTracker.stop()
--- End diff --

Is stop expected to be idempotent in this context ? Multiple invocations 
wont have an issue ?
(After this change, it can be invoked repeatedly even when trackerState == 
stopped).

This is not part of tests ... hence my query.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[mridulm]

Github user mridulm commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85435956
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -209,7 +209,8 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 
   assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
   assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
-} finally {
+} {
+  loader.close()
   Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End diff --

Might be good idea to swap these two lines - so that classloader is always 
reset even if loader.close() fails


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85109184
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

Ah-ha, sure. Let me then try. Thanks for bearing with me.


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85107727
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

Yes I think that's a good idea. Do you need the generic types on the "try" 
methods, even?


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85104721
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

Ah, did you mean something like this?

```scala
val partitioner = Utils.tryWithSafeFinally[Partitioner] {
  val deserializeStream = serializer.deserializeStream(fileInputStream)
  Utils.tryWithSafeFinally[Partitioner] {
deserializeStream.readObject[Partitioner]
  } {
deserializeStream.close()
  }
} {
  fileInputStream.close()
}
```


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85104561
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

I am sorry to ask repeatedly but could I please ask how I should change 
this a little bit more?
I was thinking I should not use `finally` upon `fileInputStream` as it 
should not always be closed at this point or around this point..



---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85046075
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

I don't mean having the finally here on this line...


---
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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/15618#discussion_r85012728
  
--- Diff: 
core/src/main/scala/org/apache/spark/rdd/ReliableCheckpointRDD.scala ---
@@ -239,7 +239,14 @@ private[spark] object ReliableCheckpointRDD extends 
Logging {
   val fs = partitionerFilePath.getFileSystem(sc.hadoopConfiguration)
   val fileInputStream = fs.open(partitionerFilePath, bufferSize)
   val serializer = SparkEnv.get.serializer.newInstance()
-  val deserializeStream = serializer.deserializeStream(fileInputStream)
+  val deserializeStream = try {
+serializer.deserializeStream(fileInputStream)
+  } catch {
+case e : Throwable =>
+  fileInputStream.close()
--- End diff --

shouldn't this be in a try {} finally {} 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 #15618: [SPARK-14914][CORE] Fix Resource not closed after...

[HyukjinKwon]

GitHub user HyukjinKwon opened a pull request:

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

[SPARK-14914][CORE] Fix Resource not closed after using, mostly for unit 
tests

## What changes were proposed in this pull request?

Close `FileStreams`, `ZipFiles` etc to release the resources after using. 
Not closing the resources will cause IO Exception to be raised while deleting 
temp files.

## How was this patch tested?

Existing tests

Closes #12693

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

$ git pull https://github.com/HyukjinKwon/spark SPARK-14914-1

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

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


commit ceb1681620b83e46425d20f20ff5a228c0a8d75b
Author: U-FAREAST\tl 
Date:   2016-04-25T03:37:06Z

mvn.cmd for windows

Helping script for windows to download dependency and start zinc to
support incremental building on windows.

commit 98ee4ab6a96b3d8ad8acd8dbeb6dedc10523546e
Author: U-FAREAST\tl 
Date:   2016-04-25T06:50:59Z

Fix file not closed on FileSuite

commit ed52ec72fee774e9b8a1bec3183ae4594d758b9f
Author: U-FAREAST\tl 
Date:   2016-04-25T07:45:39Z

Fix another file closing

commit 448508f467c0401f5461d79dadc1454b03210368
Author: Tao LI 
Date:   2016-04-25T07:57:42Z

close log data.

commit 34078a3facde46f09430baf1a1a5976b1c2d2869
Author: Tao LI 
Date:   2016-04-25T08:01:55Z

Close the class loader

commit efb7227518e9fdb8c1a35ae2adb3971c9cfc1ac2
Author: Tao LI 
Date:   2016-04-25T08:20:48Z

Another file not closed.

commit a06bffc02e4d2cd7e723e73102c160c1c57f0915
Author: Tao LI 
Date:   2016-04-25T08:29:16Z

Stop to release resources.

commit 45262dcc58073a99417b9d1a6c0e24c393716c8f
Author: U-FAREAST\tl 
Date:   2016-04-25T08:34:45Z

More closing problem

commit b3c0c96fb4cbe19344cc220e234c98644aa0efcd
Author: U-FAREAST\tl 
Date:   2016-04-25T13:25:42Z

Fix the zip file and jar file in RPackageUtilsSuite

commit a176adbadfa68cb0819a1e958129c4d96b42b42c
Author: U-FAREAST\tl 
Date:   2016-04-26T05:35:35Z

Stop ssc in MasterFailureTest

commit 35aacd29c4667550a4f870ee521ed185c5f9800c
Author: U-FAREAST\tl 
Date:   2016-04-26T08:09:04Z

Remove accidentally added files

commit 9f50128da0660fed97d64b8a5e0d63285dbf93d5
Author: U-FAREAST\tl 
Date:   2016-05-03T06:11:12Z

Code cleanup with respect to comments

commit 55b360e276968eecea970267b0fa438b56e5e703
Author: U-FAREAST\tl 
Date:   2016-05-05T03:51:50Z

Style fixes

commit 91f82b5fac48afefeceadd764ff0e7b61944d875
Author: U-FAREAST\tl 
Date:   2016-05-05T07:29:32Z

Minor code cleanup

commit f3713d1ff59e5bd45a8a207eaf36ab8e6c285812
Author: hyukjinkwon 
Date:   2016-10-25T05:44:15Z

ex -> e and indentation




---
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