-
Notifications
You must be signed in to change notification settings - Fork 883
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
Add host:port msg if we failed to initialize #303
Conversation
cc @yaooqinn |
@@ -99,7 +99,8 @@ class FrontendService private (name: String, be: BackendService, oomHook: Runnab | |||
s" [$minThreads, $maxThreads] worker threads") | |||
} catch { | |||
case e: Throwable => | |||
throw new KyuubiException("Failed to initialize frontend service", e) | |||
throw new KyuubiException("Failed to initialize frontend service, " + | |||
s"please check this host:port $serverAddr:$portNum is available.", e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like, we into a particular exception inevitable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about just Failed to initialize frontend service on host:port
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
![ulysses-you](https://badgen.net/badge/Hello/ulysses-you/green) [![PR 303](https://badgen.net/badge/Preview/PR%20303/blue)](https://github.com/yaooqinn/kyuubi/pull/303) [❨?❩](https://pullrequestbadge.com/?utm_medium=github&utm_source=yaooqinn&utm_campaign=badge_info)<!-- PR-BADGE: PLEASE DO NOT REMOVE THIS COMMENT --> <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html --> <!-- replace ${issue ID} with the actual issue id --> Fixes #303 <!-- Please clarify why the changes are needed. For instance, 1. If you add a feature, you can talk about the user case of it. 2. If you fix a bug, you can clarify why it is a bug. --> Before this PR, if we start Kyuubi with an already used port, we will get this error msg. It's better to print host:port which we bind. ``` Exception in thread "main" org.apache.kyuubi.KyuubiException: Failed to initialize frontend service at org.apache.kyuubi.service.FrontendService.initialize(FrontendService.scala:102) at org.apache.kyuubi.service.CompositeService.$anonfun$initialize$1(CompositeService.scala:40) at org.apache.kyuubi.service.CompositeService.$anonfun$initialize$1$adapted(CompositeService.scala:40) at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:62) at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:55) at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:49) at org.apache.kyuubi.service.CompositeService.initialize(CompositeService.scala:40) at org.apache.kyuubi.service.Serverable.initialize(Serverable.scala:37) at org.apache.kyuubi.server.KyuubiServer.initialize(KyuubiServer.scala:88) at org.apache.kyuubi.server.KyuubiServer$.startServer(KyuubiServer.scala:45) at org.apache.kyuubi.server.KyuubiServer$.main(KyuubiServer.scala:74) at org.apache.kyuubi.server.KyuubiServer.main(KyuubiServer.scala) Caused by: java.net.BindException: Address already in use at java.net.PlainSocketImpl.socketBind(Native Method) at java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:387) at java.net.ServerSocket.bind(ServerSocket.java:375) at java.net.ServerSocket.<init>(ServerSocket.java:237) at org.apache.kyuubi.service.FrontendService.initialize(FrontendService.scala:77) ... 11 more ``` - Add some test cases that check the changes thoroughly including negative and positive cases if possible - Add screenshots for manual tests if appropriate - [Run test](https://kyuubi.readthedocs.io/en/latest/tools/testing.html#running-tests) locally before make a pull request Squashed commit of the following: commit c3f5ffa Author: ulysses-you <ulyssesyou18@gmail.com> Date: Tue Jan 26 18:23:40 2021 +0800 fix commit 42d333d Author: ulysses-you <ulyssesyou18@gmail.com> Date: Tue Jan 26 14:44:35 2021 +0800 err msg
thanks, merged to master for v1.1.0 and branch 1.0 for v1.0.3 |
please make a followup to fix test
|
my bad.. missed the test Created #305 |
Please add issue ID here?
Fixes #303
Why are the changes needed?
Before this PR, if we start Kyuubi with an already used port, we will get this error msg.
It's better to print host:port which we bind.
Test Plan: