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

Add host:port msg if we failed to initialize #303

Closed
wants to merge 2 commits into from
Closed

Add host:port msg if we failed to initialize #303

wants to merge 2 commits into from

Conversation

ulysses-you
Copy link
Contributor

@ulysses-you ulysses-you commented Jan 26, 2021

ulysses-you Closes #303 2 1 2 Target Issue Test Plan ❨?❩

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.

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

Test Plan:

  • 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 locally before make a pull request

@ulysses-you
Copy link
Contributor Author

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)
Copy link
Member

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

Copy link
Member

@yaooqinn yaooqinn Jan 26, 2021

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@yaooqinn yaooqinn closed this in d44afdb Jan 26, 2021
yaooqinn pushed a commit that referenced this pull request Jan 26, 2021
![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) [&#10088;?&#10089;](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
@yaooqinn
Copy link
Member

thanks, merged to master for v1.1.0 and branch 1.0 for v1.0.3

@yaooqinn
Copy link
Member

please make a followup to fix test

- invalid port *** FAILED ***
  "...ize frontend service[ on fv-az60-334/10.1.0.4:0.]" did not equal "...ize frontend service[]" (ServerableSuite.scala:48)

@ulysses-you
Copy link
Contributor Author

my bad.. missed the test

Created #305

yaooqinn pushed a commit that referenced this pull request Jan 26, 2021
fix #305
Squashed commit of the following:

commit f67ca84
Author: ulysses-you <ulyssesyou18@gmail.com>
Date:   Tue Jan 26 20:36:10 2021 +0800

    init
yaooqinn pushed a commit that referenced this pull request Jan 26, 2021
fix #305
Squashed commit of the following:

commit f67ca84
Author: ulysses-you <ulyssesyou18@gmail.com>
Date:   Tue Jan 26 20:36:10 2021 +0800

    init
@yaooqinn yaooqinn added this to the v1.1.0 milestone Mar 4, 2021
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

2 participants