Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/5223#discussion_r27516465
--- Diff:
core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala ---
@@ -729,7 +729,7 @@ private[spark] class ExternalSorter[K, V, C](
var out: FileOutputStream = null
var in: FileInputStream = null
val writeStartTime = System.nanoTime
- try {
+ util.Utils.tryWithSafeFinally {
--- End diff --
I guess the same comment goes here, although I see it's kind of because
it's trying to clean up two streams but skimping on finally blocks, with just
one to do both.
But there's really no reason that the input stream has to be outside the
for loop. It should be opened and closed locally inside the loop over
partitions. So I think this can be usefully simplified along the same lines --
`out` declared outside the try block, and push `in` inside that loop.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
