You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "gaoyajun02 (via GitHub)" <gi...@apache.org> on 2023/09/20 03:11:26 UTC
[GitHub] [spark] gaoyajun02 opened a new pull request, #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
gaoyajun02 opened a new pull request, #43004:
URL: https://github.com/apache/spark/pull/43004
### What changes were proposed in this pull request?
Add inflightMergedBlocks to avoid repeated fallback when failed to fetch remote push-merged block meta
### Why are the changes needed?
When sendMergedBlockMetaReq is in flight, the channel happens to be closed, which will cause two `onFailure` callbacks, one from the responseHandler and the other from the listener of channelFuture. Two fallback will eventually cause the original block to be obtained twice.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaoyajun02 commented on pull request #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
Posted by "gaoyajun02 (via GitHub)" <gi...@apache.org>.
gaoyajun02 commented on PR #43004:
URL: https://github.com/apache/spark/pull/43004#issuecomment-1738584899
@mridulm @Ngone51 @LuciferYang
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaoyajun02 commented on pull request #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
Posted by "gaoyajun02 (via GitHub)" <gi...@apache.org>.
gaoyajun02 commented on PR #43004:
URL: https://github.com/apache/spark/pull/43004#issuecomment-1726890650
@mridulm @tgravescs @attilapiros @Ngone51 @Victsm @otterc Please help review this change.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #43004:
URL: https://github.com/apache/spark/pull/43004#issuecomment-1741906030
Wouldn't this not be the case for all uses of `RpcChannelListener.handleFailure` ? Not just for meta ?
If yes, the fix would be to handle this for `outstandingRpcs` in `TransportResponseHandler`.
Namely:
A) fix get/remove pattern:
Essentially, replace the pattern:
```
<ListenerrType> listener = (<ListenerrType>) outstandingRpcs.get(resp.requestId);
if (listener == null) {
// handle missing listener
} else {
outstandingRpcs.remove(resp.requestId);
// process
```
with:
```
<ListenerrType> listener = (<ListenerrType>) outstandingRpcs.remove(resp.requestId);
if (listener == null) {
// handle missing listener
} else {
// process
```
and
B) `removeRpcRequest` return's `boolean` indicating whether the request was removed.
C) Optionally, modify `failOutstandingRequests` to create a copy of `outstandingRpcs.values` and clear it immediately.
There is an inherent race here, but it no different from what currently exists anyway.
Thoughts ?
+CC @Ngone51, @otterc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
URL: https://github.com/apache/spark/pull/43004
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaoyajun02 commented on pull request #43004: [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta
Posted by "gaoyajun02 (via GitHub)" <gi...@apache.org>.
gaoyajun02 commented on PR #43004:
URL: https://github.com/apache/spark/pull/43004#issuecomment-1726832193
In fact, there is another fix, which is to add a check for outstandingRpcs in handleFailure of the network module.
https://github.com/apache/spark/blob/227e262025229a67f43a8de452215053a9cbf662/common/network-common/src/main/java/org/apache/spark/network/client/TransportClient.java#L391
but I am not sure whether it will affect other behaviors.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45134][SHUFFLE] Avoid repeated fallback when failed to fetch remote push-merged block meta [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #43004:
URL: https://github.com/apache/spark/pull/43004#issuecomment-1883996921
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org