LuciferYang commented on code in PR #56564:
URL: https://github.com/apache/spark/pull/56564#discussion_r3433553003
##########
core/src/main/scala/org/apache/spark/shuffle/IndexShuffleBlockResolver.scala:
##########
@@ -576,22 +576,21 @@ private[spark] class IndexShuffleBlockResolver(
private[shuffle] def getChecksums(checksumFile: File, blockNum: Int):
Array[Long] = {
if (!checksumFile.exists()) return null
- val checksums = new ArrayBuffer[Long]
// Read the checksums of blocks
- var in: DataInputStream = null
try {
- in = new DataInputStream(new NioBufferedFileInputStream(checksumFile))
- while (checksums.size < blockNum) {
- checksums += in.readLong()
+ Utils.tryWithResource {
+ new DataInputStream(new NioBufferedFileInputStream(checksumFile))
+ } { in =>
+ val checksums = new ArrayBuffer[Long]
+ while (checksums.size < blockNum) {
+ checksums += in.readLong()
+ }
+ checksums.toArray
}
} catch {
case _: IOException | _: EOFException =>
- return null
- } finally {
- in.close()
Review Comment:
Good catch. Suppressing it should be fine here: `getChecksums` reads all the
checksums before `close()`, so a `close()` failure doesn't affect the result,
and the single caller uses it best-effort (reuse, or regenerate on null). It
also matches `getMergedBlockData` in this file, which already reads via
`Utils.tryWithResource` (i.e. `closeQuietly`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
