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

Have Exponential backOff on Auth retry #232

Open
Aarati opened this issue Nov 15, 2016 · 4 comments
Open

Have Exponential backOff on Auth retry #232

Aarati opened this issue Nov 15, 2016 · 4 comments
Labels
enhancement New feature or improved functionality.

Comments

@Aarati
Copy link

Aarati commented Nov 15, 2016

Repro:
Use AuthCallback for Ably authentication
Send 404 or invalid token
Check logs

Actual:
In logs we see Auth service is tries ~30sec.
The connection goes in disconnected state right after the auth failure.
Even after 10+ mins, the connection remains in disconnected state and rest client keeps trying to do auth.

Expected:
For client performance, auth retries should be done with exponential backoff.

┆Issue is synchronized with this Jira Task by Unito

@mattheworiordan
Copy link
Member

@Aarati we have discussed exponential back-off many times, but decided on what we think is a suitable back off strategy which only comes in two forms, see https://www.ably.io/documentation/realtime/connection#connection-states for more details.

The problem we have is that your use case may not match with someone else's use case, and as such, what we instead aim to do is provide you with the means to implement your own strategy if you wish.

The solution therefore, I believe, is:

  • you implement your own logic in authCallback and can thus implement back-off if you want
  • if you want to change the retry timeouts for the client library based on the disconnected and suspended states, you can configure high values in ClientOptions (see http://docs.ably.io/client-lib-development-guide/features/#TO3l for full details), and then implement your own retry policy with backoff that would kick in before the library retries.

The bad news in this front is that the configurable retry attributes are not support in this version of ably-java, but are in ably-ios. If you want this feature prioritise, it's a trivial change and we could prioritise this?

@SimonWoolf
Copy link
Member

I agree with Matt. Exponential backoff is great for congestion avoidance, but AFAICT there's no suggestion that that's an issue here. I don't follow the "for client performance" reasoning: if some has an e.g. android app open and trying to connect, we don't want the wait between retries to increase exponentially -- if they're not connected, presumably some crucial part of the app's functionality is going to be broken. And when the app isn't active, it should not be retrying at all (the connection should have been explicitly closed on being backgrounded - we don't want to have active connections open for background apps anyway for battery reasons, that's what push notifications are for).

The only reasoning I could see for this is to prevent an auth server from being overloaded if it has to refuse token requests from everyone. The solution to that is to have the auth server return some value that you can catch in your authCallback code and close() the connection -- that way the client will just stop retrying until connect() is explicitly called again. (Or if using authUrl where you don't have a custom handler, once ably/docs#194 is finalised, you can use a 403 response for this).

@paddybyers
Copy link
Member

I think the main point is that, even with an exponential backoff approach, you don't keep increasing the interval indefinitely. There is some maximum interval reached, at which point the retries simply become periodic.

What happens in the time before that maximum interval is reached is really just fine-tuning and optimisation for short-term failures and isn't relevant for the long-term condition.

In our approach, that long-term periodic state is the suspended state. It happens that we configure the max interval by default to 30s, but that can be varied (in 0.9). We think that you would remain in the suspended state (instead of just closing the connection) if you want to take advantage automatically of connectivity being restored some time in the future, and 30s is a reasonable default taking into account the desire to reestablish connectivity relatively promptly, and the likely load on the client and systems from the periodic retries.

@mattheworiordan mattheworiordan added the enhancement New feature or improved functionality. label Nov 23, 2016
@mattheworiordan
Copy link
Member

Please note that we are reviewing this as part of a specification change, and will update this accordingly once we've agreed that. See ably/specification#83

@mattheworiordan mattheworiordan added feature under review and removed enhancement New feature or improved functionality. labels Nov 23, 2016
@ably-sync-bot ably-sync-bot changed the title Have Exponential backOff on Auth retry Have Exponential backOff on Auth retry Feb 18, 2021
@QuintinWillison QuintinWillison added enhancement New feature or improved functionality. and removed feature under review labels Dec 6, 2022
@sync-by-unito sync-by-unito bot removed the enhancement New feature or improved functionality. label Feb 23, 2023
@ttypic ttypic added the enhancement New feature or improved functionality. label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improved functionality.
Development

No branches or pull requests

6 participants