Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9455#discussion_r43830137
--- Diff:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeSorterSpillReader.java
---
@@ -31,10 +30,8 @@
* Reads spill files written by {@link UnsafeSorterSpillWriter} (see that
class for a description
* of the file format).
*/
-public final class UnsafeSorterSpillReader extends UnsafeSorterIterator {
- private static final Logger logger =
LoggerFactory.getLogger(UnsafeSorterSpillReader.class);
+public final class UnsafeSorterSpillReader extends UnsafeSorterIterator
implements Closeable {
--- End diff --
Of the potential leaks that were discovered, I think that this is the only
one to be concerned about: it looks like `UnsafeSorterSpillReader` might leak
an open `FileInputStream` if an exception occurred while reading records. The
fix implemented here is to add a `close()` method for safely closing the
reader's streams.
I chose to move the spill deletion logic out of the reader itself since it
appears to be redundant with spill deletion code that lives elsewhere. We
should audit the existing code to make sure that the chain of responsibility
for cleaning up spill files is clearly defined.
---
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
