Ngone51 commented on a change in pull request #32287:
URL: https://github.com/apache/spark/pull/32287#discussion_r622698006
##########
File path:
core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala
##########
@@ -613,6 +618,12 @@ final class ShuffleBlockFetcherIterator(
}
if (isNetworkReqDone) {
reqsInFlight -= 1
+ if (!buf.isInstanceOf[NettyManagedBuffer]) {
+ // Non-`NettyManagedBuffer` doesn't occupy Netty's memory so we
can unset the flag
+ // directly once the request succeeds. But for the
`NettyManagedBuffer`, we'll only
+ // unset the flag when the data is fully consumed (see
`BufferReleasingInputStream`).
+ NettyUtils.isNettyOOMOnShuffle = false
Review comment:
So assume the avgA is for the iterator A and avgB is for the iterator B
and we're going to unset the flag when freeMemory > avgA:
If avgA > avgB, I think this shouldn't be a problem for iterator B.
If avgA < avgB, although flag is unset, but please note that iterator B
won't start the next request until an in-flight request finished, which would
release the avgB memory. So I think it should also be fine.
BTW, it could be complex if we want to maintain a global min average. For
example, if the current global min average is avgA and iterator A finished, we
need to update the global min average, which can be complex to compute at this
time.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
