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

[MINOR] fix(mr): fix default value #1035

Merged
merged 2 commits into from
Aug 2, 2023
Merged

Conversation

zhengchenyu
Copy link
Collaborator

What changes were proposed in this pull request?

When configured as README.md, MapReduce will throw these errors. It is not helpful for beginner.

Exception in thread "main" java.lang.IllegalArgumentException: mapreduce.rss.client.retry.max(100) * mapreduce.rss.client.retry.interval.max(10000) should not bigger than mapreduce.rss.client.send.check.timeout.ms(600000)
	at org.apache.hadoop.mapreduce.RssMRUtils.validateRssClientConf(RssMRUtils.java:307)
	at org.apache.hadoop.mapreduce.v2.app.RssMRAppMaster.main(RssMRAppMaster.java:189)

and

Exception in thread "main" java.lang.IllegalArgumentException: The value of null should be one of [GRPC_NETTY, GRPC]
	at org.apache.uniffle.client.util.ClientUtils.validateClientType(ClientUtils.java:144)
	at org.apache.hadoop.mapreduce.v2.app.RssMRAppMaster.main(RssMRAppMaster.java:141)

How was this patch tested?

test in yarn cluster.

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2023

Codecov Report

Merging #1035 (932c486) into master (ecfed5e) will increase coverage by 2.50%.
Report is 14 commits behind head on master.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1035      +/-   ##
============================================
+ Coverage     54.10%   56.60%   +2.50%     
+ Complexity     2547     2078     -469     
============================================
  Files           382      317      -65     
  Lines         21728    13756    -7972     
  Branches       1802     1224     -578     
============================================
- Hits          11755     7786    -3969     
+ Misses         9269     5555    -3714     
+ Partials        704      415     -289     

see 100 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jerqi jerqi changed the title [MINOR] mr: fix default value [MINOR] fix(mr): fix default value Jul 26, 2023
@jerqi
Copy link
Contributor

jerqi commented Aug 2, 2023

@zhengchenyu Could you run the command to fix style issues?

mvn spotless:apply -Pspark3 -Pspark2 -Ptez -Pmr -Phadoop2.8

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merged to master. Thanks @zhengchenyu

@jerqi jerqi merged commit 54c81f0 into apache:master Aug 2, 2023
30 checks passed
@zhengchenyu zhengchenyu deleted the fix.default branch August 3, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants