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

Issue 5008 sporadic failures due used tcp ports #7008

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

Conversation

kuldeepk3
Copy link
Contributor

@kuldeepk3 kuldeepk3 commented Mar 6, 2023

Change log description
This pr addresses Sporadic failures due used tcp ports in case there is failure while starting bookeeper and goes for a retry.

Purpose of the change
Fixes #5008

What the code does
Adding retry in case bookkeeper fails to start with address already in use . This change will retry the creation of bookkeeper in case it fails to start with BindException.

How to verify it
Execute the unit test and there should be no failure with exception: java.net.BindException: Address already in use

…in use

Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
@kuldeepk3 kuldeepk3 marked this pull request as ready for review March 6, 2023 10:03
@krishan-Rai krishan-Rai self-requested a review March 6, 2023 10:38
Copy link

@krishan-Rai krishan-Rai left a comment

Choose a reason for hiding this comment

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

LGTM

@abhinb
Copy link
Contributor

abhinb commented Mar 6, 2023

Could you test this by checking if you can make the test start on a already occupied port and then see if the retry logic works?

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Patch coverage: 66.03% and project coverage change: -0.01 ⚠️

Comparison is base (65ac2c1) 86.33% compared to head (d910d5d) 86.32%.

❗ Current head d910d5d differs from pull request most recent head 73aa38b. Consider uploading reports for the commit 73aa38b to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #7008      +/-   ##
============================================
- Coverage     86.33%   86.32%   -0.01%     
+ Complexity    16048    16002      -46     
============================================
  Files          1031     1030       -1     
  Lines         59815    59543     -272     
  Branches       6071     6024      -47     
============================================
- Hits          51639    51402     -237     
+ Misses         4998     4991       -7     
+ Partials       3178     3150      -28     
Impacted Files Coverage Δ
.../main/java/io/pravega/common/util/CommonUtils.java 35.08% <0.00%> (ø)
...orage/impl/bookkeeper/BookKeeperServiceRunner.java 70.22% <67.30%> (-2.56%) ⬇️

... and 29 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@RaulGracia RaulGracia left a comment

Choose a reason for hiding this comment

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

I understand the approach, but we really need that verify this works.

bookieServerAndResources = runBookie(bkPort);
retry = false;
} catch (BindException e) {
if ( ++count >= retries ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra head and tail spaces.

kuldeepk3 and others added 10 commits March 20, 2023 00:53
…t/common/TestUtils.java -> common/src/main/java/io/pravega/common/util/CommonUtils.java

Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
Signed-off-by: Kuldeep Kumar <kuldeep.kumar3@dell.com>
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.

Sporadic failures due used tcp ports
7 participants