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.

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

[Improvement] [mr] Ensure configurations in both mapred-site.xml and dynamic_client.conf take effect. #1051

Closed
3 tasks done
qijiale76 opened this issue Jul 28, 2023 · 1 comment · Fixed by #1112
Closed
3 tasks done
Assignees
Labels
0.8 For 0.8 version release

Comments

@qijiale76
Copy link
Contributor

Code of Conduct

Search before asking

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

What would you like to be improved?

If dynamic conf is not enabled, though mapreduce.rss.storage.type is configued at mapred-site.xml, running MR job within RSS still fails, with this error message Error: java.lang.IllegalArgumentException: No enum constant org.apache.uniffle.storage.util.StorageType.
The reason is only rssJobConf is used to new CreateShuffleReadClientRequest


This issue will optimize how configuration works. Configuration in both dynamic conf and local conf file should work.

How should we improve?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@qijiale76
Copy link
Contributor Author

Now mapreduce.rss.client.type and mapreduce.rss.remote.storage.path must be configued at mapred-site.xml instead of dynamic_client.conf. It should also be allowd that these conf are not configued at mapred-site.xml.

Exception in thread "main" java.lang.IllegalArgumentException: The value of null should be one of [GRPC_NETTY, GRPC]
Exception in thread "main" java.lang.IllegalStateException: Can't find remoteStorage: with storageType[MEMORY_LOCALFILE_HDFS]

@qijiale76 qijiale76 changed the title [Improvement] [mr] Merge mrJobConf and rssJobConf at RssShuffle.java [Improvement] [mr] Ensure configurations in both mapred-site.xml and dynamic_client.conf take effect. Jul 28, 2023
@jerqi jerqi added the 0.8 For 0.8 version release label Jul 29, 2023
jerqi pushed a commit that referenced this issue Aug 17, 2023
…namic_client.conf take effect. (#1112)

### What changes were proposed in this pull request?
Ensure configurations in both mapred-site.xml and dynamic_client.conf take effect.

### Why are the changes needed?
Fix: #1051

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

No.

### How was this patch tested?

tested in our cluster and UT

Co-authored-by: qijiale <qijiale001@ke.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.8 For 0.8 version release
Projects
None yet
2 participants