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

Revising connection retry config #15923

Merged

Conversation

@sancar
Copy link
Member

sancar commented Nov 1, 2019

ConnectionRetryConfig was not allowing retry with fixed amount of time
and shutdown after some time.

Deprecated/Removed connectionAttemptLimit and period was solution
to this.

In 4.0 since we want single place to configure this,
ConnectionRetryConfig is modified to support both exponential and
fixed wait time.

To achieve this:
fail-on-maxbackooff is removed
cluster-connect-timeout-seconds is added.(default is 20)

@sancar sancar added this to the 4.0 milestone Nov 1, 2019
@sancar sancar requested a review from hazelcast/clients as a code owner Nov 1, 2019
@sancar sancar self-assigned this Nov 1, 2019
@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch from b7d6623 to 0f82513 Nov 1, 2019
@sancar

This comment has been minimized.

Copy link
Member Author

sancar commented Nov 6, 2019

run-lab-run

1 similar comment
@sancar

This comment has been minimized.

Copy link
Member Author

sancar commented Nov 7, 2019

run-lab-run

@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch from 0f82513 to 8ec8c87 Nov 7, 2019
Copy link
Contributor

ihsandemir left a comment

attempt-limit is what we were used to in the older clients but I believe for the user's perspective, what is more valuable is "total time to connect to a candidate cluster". Hence, I would rather prefer using a setting such as "max-candidate-cluster-connect-duration" rather than attempt limit. I find it more difficult if I use backoff strategy, then I need to do a complex calculation to set the attempt limit to set a max time limit of let's say 30 secs. I find using duration easier to use, more useful and more user friendly.

@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch 3 times, most recently from fcc19a5 to 9c73f7d Nov 11, 2019
logger.warning(String.format("Unable to get live cluster connection, attempt %d.", attempt));
long currentTimeMillis = Clock.currentTimeMillis();
if (currentTimeMillis > clusterConnectDeadline) {
logger.warning(String.format("Unable to get live cluster connection, cluster connect timeout %d seconds is "

This comment has been minimized.

Copy link
@ihsandemir

ihsandemir Nov 12, 2019

Contributor

re-wording timeout --> of timeout


logger.warning(String.format("Unable to get live cluster connection, retry in %d ms, attempt %d "
+ ", cluster connect timeout seconds %d "
+ ", retry timeout millis %d cap", actualSleepTime, attempt, clusterConnectTimeoutSeconds, maxBackoffMillis));

This comment has been minimized.

Copy link
@ihsandemir

ihsandemir Nov 12, 2019

Contributor

can you use a more understandable word other than cap, e.g. maximum retry timeout millis

@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch from 9c73f7d to fbeceea Nov 12, 2019
@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch 2 times, most recently from 9164632 to d15f944 Nov 21, 2019
@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch from d15f944 to 8d98c53 Nov 21, 2019
ConnectionRetryConfig was not allowing retry with fixed amount of time
and shutdown after some time.

Deprecated/Removed connectionAttemptLimit and period was solution
to this.

In 4.0 since we want single place to configure this,
ConnectionRetryConfig is modified to support both exponential and
fixed wait time.

To achieve this:
fail-on-maxbackooff is removed
cluster-connect-timeout-seconds is added.(default is 20)
@sancar sancar force-pushed the sancar:fix/reviseConnectionRetryConfig/master branch from 8d98c53 to d2bbe48 Nov 21, 2019
@sancar sancar merged commit c89eada into hazelcast:master Nov 22, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@sancar sancar deleted the sancar:fix/reviseConnectionRetryConfig/master branch Nov 22, 2019
sancar added a commit to sancar/hazelcast-code-samples that referenced this pull request Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.