Victsm commented on a change in pull request #30433:
URL: https://github.com/apache/spark/pull/30433#discussion_r539614385
##########
File path:
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -586,6 +592,7 @@ public void onData(String streamId, ByteBuffer buf) throws
IOException {
deferredBufs = null;
return;
}
+ abortIfNecessary();
Review comment:
The way I see the issue mentioned in that comment is that we are not
preventing new blocks to be merged when the IOException threshold is reached.
To do that, we only need to invoke `abortIfNecessary` inside `onComplete`,
whether we still have any deferredBuf to write at that point.
This way, for normal case without IOException, we are only invoking
`abortIfNecessary` once per block.
By invoking it here, we would invoke it once per buf for normal case.
Of course, if we only check inside `onComplete`, we would delay rejection of
these pending blocks until we reach their stream's end.
I think this is a reasonable tradeoff to make, considering that majority of
the time the code is executing for normal case instead of the exception case.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
