You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "huangxiaopingRD (via GitHub)" <gi...@apache.org> on 2025/11/26 12:17:10 UTC
[PR] [SPARK-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java` [spark]
huangxiaopingRD opened a new pull request, #53231:
URL: https://github.com/apache/spark/pull/53231
### What changes were proposed in this pull request?
Remove `getRecoveryPath` method from `YarnShuffleService.java`, it is no longer used.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No need test
### 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
Re: [PR] [SPARK-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java` [spark]
Posted by "huangxiaopingRD (via GitHub)" <gi...@apache.org>.
huangxiaopingRD commented on PR #53231:
URL: https://github.com/apache/spark/pull/53231#issuecomment-3587690764
> However, we maintained both setter and getter methods like Apache Hadoop `AuxiliaryService` abstract class. I'm not sure it's a good idea for us to remove the only way to get the recovery path at Apache Spark 4.2.0.
>
> ```java
> public void setRecoveryPath(Path recoveryPath) {
> protected Path getRecoveryPath(String fileName) {
> ```
>
> IIUC, this is public class, isn't it?
>
> https://github.com/apache/spark/blob/154a2708e98840ba4d69b5afdd549e695081e4ce/common/network-yarn/src/main/java/org/apache/spark/network/yarn/YarnShuffleService.java#L101
I understand what you mean. Thank you @dongjoon-hyun
--
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-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java` [spark]
Posted by "huangxiaopingRD (via GitHub)" <gi...@apache.org>.
huangxiaopingRD commented on PR #53231:
URL: https://github.com/apache/spark/pull/53231#issuecomment-3583937445
> May I ask why you think like that, @huangxiaopingRD ?
>
> > Remove getRecoveryPath method from YarnShuffleService.java, it is no longer used.
@dongjoon-hyun Thank you for your reply. After this PR (https://github.com/apache/spark/pull/14999/files), `getRecoveryPath` is not used anywhere else.
--
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-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java` [spark]
Posted by "huangxiaopingRD (via GitHub)" <gi...@apache.org>.
huangxiaopingRD closed pull request #53231: [SPARK-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java`
URL: https://github.com/apache/spark/pull/53231
--
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-54527][CORE] Remove `getRecoveryPath` method from `YarnShuffleService.java` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #53231:
URL: https://github.com/apache/spark/pull/53231#issuecomment-3587584104
However, we maintained both setter and getter methods like Apache Hadoop `AuxiliaryService` abstract class. I'm not sure it's a good idea for us to remove the only way to get the recovery path at Apache Spark 4.2.0.
```java
public void setRecoveryPath(Path recoveryPath) {
protected Path getRecoveryPath(String fileName) {
```
IIUC, this is public class, isn't it?
https://github.com/apache/spark/blob/154a2708e98840ba4d69b5afdd549e695081e4ce/common/network-yarn/src/main/java/org/apache/spark/network/yarn/YarnShuffleService.java#L101
--
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