otterc commented on a change in pull request #30433:
URL: https://github.com/apache/spark/pull/30433#discussion_r539500739
##########
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:
@Victsm This change is to address this comment:
https://github.com/apache/spark/pull/30433#discussion_r538947833
The scenario is that there can be pending streams which are waiting on the
lock for the `partitionInfo` and meanwhile the exception threshold has met.
When the pending stream acquires the lock it will attempt to write to the data
file even though the exception threshold is reached.
I have added the UT `testPendingBlockIsAbortedImmediately` to verify this.
> Purpose is to only do this check once per block instead of once per buf
and avoid throwing unnecessary exceptions from onData which leads to channel
close.
We already throw IOExceptions when write to data file fails. I don't see how
throwing `IOException exceeded threshold` makes it any worse.
----------------------------------------------------------------
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]
