otterc commented on a change in pull request #30433:
URL: https://github.com/apache/spark/pull/30433#discussion_r539871040
##########
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:
Even If the write succeeds, I don't see an issue. If there are no
further failures with this block it will be merged successfully. If there are
more failures later, then they will be handled in the same way as now. I could
add UTs to verify this as well.
> we still receive data in onData but just not write when the partition
already reach IO threshold and only call abortIfNecessary inside `onComplete`.
My thought about this is that if a block is completing and doesn't have any
deferred bufs then we should let it complete without failures if possible.
Again, I don't have a strong opinion about this. IMO reaching the threshold
is indicative of a persistent issue and any further writes will faill.
----------------------------------------------------------------
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]
