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

[#1735] fix(client): MultiException class not found when reassign or stage retry is enabled #1725

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dingshun3016
Copy link
Contributor

@dingshun3016 dingshun3016 commented May 20, 2024

What changes were proposed in this pull request?

include jetty-util and shade it

Why are the changes needed?

Fix: #1735

When reassign or stag retry is enabled, GrpcServer needs to be started. If there is a port conflict during startup and retry is performed, MultiException will be thrown and the following exception will occur, causing the job to fail.The root cause is that uniffle did not include jetty-util related packages.

ERROR [main] SparkSubmit$$anon$2: spark submit throw error: org/eclipse/jetty/util/MultiException java.lang.NoClassDefFoundError: org/eclipse/jetty/util/MultiException at org.apache.uniffle.common.util.RssUtils.isServerPortBindCollision(RssUtils.java:219) at org.apache.uniffle.common.util.RssUtils.startServiceOnPort(RssUtils.java:197) at org.apache.uniffle.common.rpc.GrpcServer.start(GrpcServer.java:188) at org.apache.spark.shuffle.RssShuffleManager.<init>(RssShuffleManager.java:262) at org.apache.spark.shuffle.DelegationRssShuffleManager.createShuffleManagerInDriver(DelegationRssShuffleManager.java:87) at org.apache.spark.shuffle.DelegationRssShuffleManager.<init>(DelegationRssShuffleManager.java:60) at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:423)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No need.

Copy link

github-actions bot commented May 20, 2024

Test Results

 2 641 files  +9   2 641 suites  +9   5h 27m 29s ⏱️ + 4m 59s
   944 tests ±0     943 ✅ + 1   1 💤 ±0  0 ❌ ±0 
11 773 runs  +9  11 758 ✅ +10  15 💤 ±0  0 ❌ ±0 

Results for commit bde6a6c. ± Comparison against base commit 4969822.

♻️ This comment has been updated with latest results.

@jerqi
Copy link
Contributor

jerqi commented May 22, 2024

@dingshun3016 Could you create an issue for this?

@zuston
Copy link
Member

zuston commented May 22, 2024

Why we need jetty package in client side? @dingshun3016

@dingshun3016 dingshun3016 changed the title fix(client): MultiException class not found when reassign or stag retry is enabled [#1735] fix(client): MultiException class not found when reassign or stag retry is enabled May 22, 2024
@dingshun3016
Copy link
Contributor Author

@dingshun3016 Could you create an issue for this?
Okay, I've created

@dingshun3016
Copy link
Contributor Author

Why we need jetty package in client side? @dingshun3016

You can take a look at the stack information above, mainly using the MultiException class in jetty to determine whether it is a port conflict exception.

@dingshun3016
Copy link
Contributor Author

I moved MultiException from the jetty package to uniffle and renamed it to RssMultiException

@dingshun3016
Copy link
Contributor Author

I moved MultiException from the jetty package to uniffle and renamed it to RssMultiException

After consideration, it seems that MultiException cannot be ported to uniffle, because the exception thrown is org/eclipse/jetty/util/MultiException

@zuston
Copy link
Member

zuston commented Jun 24, 2024

image

I think we could remove this. @dingshun3016

@dingshun3016
Copy link
Contributor Author

image

I think we could remove this. @dingshun3016

Without using the isServerPortBindCollision, if an exception occurs in startServiceOnPort, retry directly, is that right?

@rickyma rickyma changed the title [#1735] fix(client): MultiException class not found when reassign or stag retry is enabled [#1735] fix(client): MultiException class not found when reassign or stage retry is enabled Jun 29, 2024
@maobaolong
Copy link
Member

@dingshun3016 Thanks for your contributions. And I have another fix seems can resolve more conflict and CNFE, PTAL
#1878

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.

[Bug] MultiException class not found when reassign or stage retry is enabled
4 participants