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

Make min_backoff_ms timeout configurable #10237

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

rdsedmundo
Copy link
Contributor

This PR aims to make the variable min_backoff_ms configurable at NodeJS.

Currently, only the max_backoff_ms can be configured individually. And if I'd like to change the max time for a value less than 20 seconds, it didn't work, because the min_backoff_ms will always have precedence, and it's currently forced to 20 seconds.

In a nutshell, this will solve this problem:
http://stackoverflow.com/questions/42256810/how-can-i-change-grpcs-reconnection-behaviour-in-the-node-js-implementation

The answer made by @murgatroid99 didn't work because the min is always 20.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@googlebot
Copy link

CLAs look good, thanks!

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

11 similar comments
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@murgatroid99
Copy link
Member

This is OK to test

@mkvonarx
Copy link

mkvonarx commented Apr 5, 2017

What's the status of this PR? I'm also very interested in this feature.

@rdsedmundo rdsedmundo force-pushed the configurable-minbackoff-reconnect branch from 132dd1d to f6a4d9f Compare April 7, 2017 11:41
@rdsedmundo
Copy link
Contributor Author

rdsedmundo commented Apr 7, 2017

I've rebased the branch with master.

@murgatroid99 Can you assign @ctiller here? I guess it's better for him to see that.

@grpc-kokoro
Copy link

No significant performance differences

@ctiller ctiller merged commit 1eeed85 into grpc:master Apr 18, 2017
@pdf13
Copy link

pdf13 commented May 23, 2017

@ctiller can you tell me if this pull request will be released with 1.4.x milestone?

@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants