Skip to content

Conversation

@venkata91
Copy link
Contributor

What changes were proposed in this pull request?

Minor changes to change the config key name from spark.shuffle.server.mergedShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl. This is missed out in #33615.

Why are the changes needed?

To keep the config names consistent

Does this PR introduce any user-facing change?

Yes, this is a change in the config key name. But the new config name changes are yet to be released. Technically there is no user facing change because of this change.

How was this patch tested?

Existing tests.

@venkata91
Copy link
Contributor Author

cc @mridulm @Ngone51 I missed to change one of the config key to use spark.shuffle.push.server.mergedShuffleFileManagerImpl. Sorry about that.

@mridulm
Copy link
Contributor

mridulm commented Aug 20, 2021

Ok to test

@mridulm
Copy link
Contributor

mridulm commented Aug 20, 2021

+CC @gengliangwang, @Ngone51

We should get this into branch-3.2 as well.

@SparkQA
Copy link

SparkQA commented Aug 20, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47177/

@SparkQA
Copy link

SparkQA commented Aug 20, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47177/

@SparkQA
Copy link

SparkQA commented Aug 20, 2021

Test build #142676 has finished for PR 33799 at commit f70db3f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon HyukjinKwon changed the title [SPARK-36374][FOLLOW-UP]Change config key spark.shuffle.server.mergedShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl [SPARK-36374][FOLLOW-UP] Change config key spark.shuffle.server.mergedShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl Aug 22, 2021
@HyukjinKwon
Copy link
Member

We should technically wait for RC 1 to fail before merging .... although typically RC1 would very very likely fail .. :-).

@mridulm
Copy link
Contributor

mridulm commented Aug 22, 2021

RC.1 does not include #33790, which results in test failure - so it will unfortunately fail.

@asfgit asfgit closed this in 7b2842e Aug 22, 2021
asfgit pushed a commit that referenced this pull request Aug 22, 2021
…dShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl

### What changes were proposed in this pull request?

Minor changes to change the config key name from `spark.shuffle.server.mergedShuffleFileManagerImpl` to `spark.shuffle.push.server.mergedShuffleFileManagerImpl`. This is missed out in #33615.

### Why are the changes needed?

To keep the config names consistent

### Does this PR introduce _any_ user-facing change?

Yes, this is a change in the config key name. But the new config name changes are yet to be released. Technically there is no user facing change because of this change.

### How was this patch tested?

Existing tests.

Closes #33799 from venkata91/SPARK-36374-follow-up.

Authored-by: Venkata krishnan Sowrirajan <vsowrirajan@linkedin.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
(cherry picked from commit 7b2842e)
Signed-off-by: Mridul Muralidharan <mridulatgmail.com>
@mridulm
Copy link
Contributor

mridulm commented Aug 22, 2021

Merged to master and branch-3.2
+CC @gengliangwang

Thanks for fixing this @venkata91 !
Thanks for the review @HyukjinKwon :-)

@gengliangwang
Copy link
Member

Late +1

venkata91 added a commit to linkedin/spark that referenced this pull request Oct 13, 2021
…dShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl

Minor changes to change the config key name from `spark.shuffle.server.mergedShuffleFileManagerImpl` to `spark.shuffle.push.server.mergedShuffleFileManagerImpl`. This is missed out in apache#33615.

To keep the config names consistent

Yes, this is a change in the config key name. But the new config name changes are yet to be released. Technically there is no user facing change because of this change.

Existing tests.

Closes apache#33799 from venkata91/SPARK-36374-follow-up.

Authored-by: Venkata krishnan Sowrirajan <vsowrirajan@linkedin.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
(cherry picked from commit 7b2842e)
Signed-off-by: Mridul Muralidharan <mridulatgmail.com>
wangyum pushed a commit that referenced this pull request May 26, 2023
…dShuffleFileManagerImpl to spark.shuffle.push.server.mergedShuffleFileManagerImpl

Minor changes to change the config key name from `spark.shuffle.server.mergedShuffleFileManagerImpl` to `spark.shuffle.push.server.mergedShuffleFileManagerImpl`. This is missed out in #33615.

To keep the config names consistent

Yes, this is a change in the config key name. But the new config name changes are yet to be released. Technically there is no user facing change because of this change.

Existing tests.

Closes #33799 from venkata91/SPARK-36374-follow-up.

Authored-by: Venkata krishnan Sowrirajan <vsowrirajan@linkedin.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants