[SPARK-19450] Replace askWithRetry with askSync. #16790
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
#16690 (comment) causes the build to produce lots of deprecation warnings. |
|
If it was just an internal API that's not supposed to be used not, and |
|
@srowen Both |
|
ok to test |
|
The change LGTM pending tests. |
|
Test build #73069 has finished for PR 16790 at commit
|
|
retest this please |
|
Test build #73084 has finished for PR 16790 at commit
|
|
Merged to master |
Yunni
pushed a commit
to Yunni/spark
that referenced
this pull request
Feb 27, 2017
## What changes were proposed in this pull request? `askSync` is already added in `RpcEndpointRef` (see SPARK-19347 and apache#16690 (comment)) and `askWithRetry` is marked as deprecated. As mentioned SPARK-18113(apache#16503 (comment)): >askWithRetry is basically an unneeded API, and a leftover from the akka days that doesn't make sense anymore. It's prone to cause deadlocks (exactly because it's blocking), it imposes restrictions on the caller (e.g. idempotency) and other things that people generally don't pay that much attention to when using it. Since `askWithRetry` is just used inside spark and not in user logic. It might make sense to replace all of them with `askSync`. ## How was this patch tested? This PR doesn't change code logic, existing unit test can cover. Author: jinxing <[email protected]> Closes apache#16790 from jinxing64/SPARK-19450.
dongjoon-hyun
pushed a commit
that referenced
this pull request
May 24, 2022
…rpc.retry.wait` configs ### What changes were proposed in this pull request? This PR cleans up the `spark.rpc.numRetries` and `spark.rpc.retry.wait` configs, both of which are unused. ### Why are the changes needed? Since SPARK-19450 / #16790 in Spark 2.2.0, both of these configurations are unused and setting them has no effect. Marking the configs as deprecated and cleaning them up from the docs helps to avoid user confusion. In addition, this cleanup slightly improves the performance of constructing `RpcEndpointRef`s because it removes two unused fields that were initialized by reading the deprecated configs. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? n/a Closes #36645 from JoshRosen/cleanup-unused-rpc-configs. Authored-by: Josh Rosen <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
askSyncis already added inRpcEndpointRef(see SPARK-19347 and #16690 (comment)) andaskWithRetryis marked as deprecated.As mentioned SPARK-18113(#16503 (comment)):
Since
askWithRetryis just used inside spark and not in user logic. It might make sense to replace all of them withaskSync.How was this patch tested?
This PR doesn't change code logic, existing unit test can cover.