mridulm commented on a change in pull request #33896:
URL: https://github.com/apache/spark/pull/33896#discussion_r747054244
##########
File path: core/src/main/scala/org/apache/spark/shuffle/ShuffleBlockPusher.scala
##########
@@ -327,10 +338,32 @@ private[spark] class ShuffleBlockPusher(conf: SparkConf)
extends Logging {
s"stop.")
return false
} else {
+ if (remainingBlocks.isEmpty && pushRequests.isEmpty &&
deferredPushRequests.isEmpty) {
Review comment:
`pushUpToMax` invokes `sendRequest` in multiple cases - each with a
different `PushRequest`.
`remainingBlocks` is based on the current `PushRequest` - the remaining
blocks for that push request.
Consider @Ngone51's example - suppose we have 2 total push requests -
request1 and request2.
If we have request1 and request2 getting pushed concurrently (from
`pushUpToMax`), completion of request1 does not mean all push has completed -
request2 is still ongoing.
In this scenario, `pushRequests` and `deferredPushRequests` would be empty -
since request1 and request2 were started together.
We will have `remainingBlocks` for request1 empty - but the concurrent
request2 would not have completed.
See if we can leverage `reqsInFlight` instead.
Hopefully this clarifies.
--
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]
