Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Improvement] Set rss.enabled to true in RssShuffleManager #2192

Closed
3 tasks done
wForget opened this issue Oct 16, 2024 · 0 comments · Fixed by #2193
Closed
3 tasks done

[Improvement] Set rss.enabled to true in RssShuffleManager #2192

wForget opened this issue Oct 16, 2024 · 0 comments · Fixed by #2193

Comments

@wForget
Copy link
Member

wForget commented Oct 16, 2024 •
edited
Loading

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

Currently, uniffle determines whether reliable storage is supported based on rss.enable configuration and shuffle manager class.

incubator-uniffle/client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleDriverComponents.java

Lines 37 to 42 in a362612

public boolean supportsReliableStorage() {
return sparkConf.get(RssSparkConfig.RSS_ENABLED)
|| RssShuffleManager.class
.getCanonicalName()
.equals(sparkConf.get("spark.shuffle.manager"));
}

This makes RssShuffleManager subclasses like UniffleShuffleManager in gluten not be considered to support reliable storage, causing DRA to not work, see apache/incubator-gluten#7559 .

Can we set rss.enabled to true in RssShuffleManager?

How should we improve?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@wForget wForget mentioned this issue Oct 16, 2024
Merged
@zuston zuston closed this as completed in e6e7819 Oct 16, 2024
zuston pushed a commit to zuston/incubator-uniffle that referenced this issue Dec 9, 2024
@wForget
[apache#2192] feat(spark3): Explicitly set rss.enabled as true to mak…
a0ce5d9 
…e ShuffleDataIO valid (apache#2193)

set rss.enabled to true in RssShuffleManager

Fix: apache#2192

No.

Neen't
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#2192] improvement(spark): set rss.enabled to true in RssShuffleManager wForget/incubator-uniffle
1 participant
@wForget