mridulm commented on a change in pull request #33451:
URL: https://github.com/apache/spark/pull/33451#discussion_r677667784



##########
File path: 
core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala
##########
@@ -822,8 +836,15 @@ final class ShuffleBlockFetcherIterator(
               }
             } catch {
               case e: IOException =>
-                buf.release()
+                // When shuffle checksum is enabled, for a block that is 
corrupted twice,
+                // we'd calculate the checksum of the block by consuming the 
remaining data
+                // in the buf. So, we should release the buf later.
+                if (!(checksumEnabled && corruptedBlocks.contains(blockId))) {
+                  buf.release()
+                }

Review comment:
       I phrased it poorly, thanks for clarifying and the added context around 
`BufferReleasingInputStream` - that is what I was trying to get to: "To 
clarify, with this change, for all fetches after the first failure, we will 
diagnose and retry only if cause != disk"




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

Reply via email to