[HUDI-8508] Fix timeline service port assignment bug #12241
Conversation
| app.start(tryPort); | ||
| return app.port(); | ||
| } catch (Exception e) { | ||
| if (e.getMessage() != null && e.getMessage().contains("Failed to bind to")) { |
There was a problem hiding this comment.
Is this line an error since it is introduced, is there any behavior change for Javaline itself?
There was a problem hiding this comment.
This is just a better way to do error classification. If a developer in Javalin changes the string then your handling will break so that is why you typically catch a specific exception type instead of simply Exception.
The bug is up at line 348. The underlying server object is the same and the code tries to call start on it twice and the underlying port does not change.
There was a problem hiding this comment.
The bug is up at line 348
I didn't quite get it, the original logic invoke startService only once, where a singleton Javaline app would be created, and the startServiceOnPort would try to start a port on the app then returns, so the actual port would be updated.
There was a problem hiding this comment.
You can take the test case and run it against master to better see the issue. There is a server object that is passed in to Javalin that is not updated and will have the old port set.
There was a problem hiding this comment.
It looks like the server started by Javaline is kind of sinlgeton within one JVM process, and the app can not start twice if the server already got started on a port. Do we have a scenario that multiple timeline server got triggered to start in one JVM?
There was a problem hiding this comment.
This logic can be hit if the port is already in use on the machine
Can you supplement the test case to cover this case?
There was a problem hiding this comment.
The test case is doing this currently, can you be more specific about what you're looking for?
There was a problem hiding this comment.
This logic can be hit if the port is already in use on the machine
I'm not convinced by this, the used port is occupied by the Javaline app itselft, I'm wondering how it behaves if the port is assigned by stuff that is not Javaline.
There was a problem hiding this comment.
ok, I will add another case with some lightweight server option
There was a problem hiding this comment.
Added a new test
github-actions
bot
added
size:M
Change Logs
Impact
Fixes existing bug in timeline server code
Risk level (write none, low medium or high below)
Low
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist