Ngone51 commented on pull request #31942: URL: https://github.com/apache/spark/pull/31942#issuecomment-809035511
I'm also confused with this part. I don't even see a place where the `resp.body()` (a.k.a `ManagedBuffer`) is referenced before the `TransportResponseHandler` handle the `ResponseMessage`. And in the case of `ChunkFetchSuccess`, I wonder we may release the buffer here too early since the `listener.onSuccess(...)` is executed asynchronously: https://github.com/apache/spark/blob/4b9e94c44412f399ba19e0ea90525d346942bf71/common/network-common/src/main/java/org/apache/spark/network/client/TransportResponseHandler.java#L162-L173 Another possible issue is, the buffer returned by `ChunkFetchSuccess` is supposed to be released after the data has been consumed: https://github.com/apache/spark/blob/2356cdd420f600f38d0e786dc50c15f2603b7ff2/core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala#L257-L259 but seems like we now only release the buffer when exception throws during buffer reading. And for a normally consumed buffer, we seem to forget to release it. cc @mridulm @tgravescs @attilapiros Do you have any idea? -- 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
