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

[FIX] web: add exponential backoff strategy for lost connection #30136

Closed
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
5 participants
@ged-odoo
Copy link
Contributor

ged-odoo commented Jan 11, 2019

Before this commit, the web client had a naive strategy to handle lost
connections: it tried to poll the server every 2 seconds until a rpc
succeeds.

This works quite well from the perspective of the user, but may be a problem
from the perspective of the server. If a server is down for a longish period,
then each users active tabs will then perform a request every 2 seconds. This
means that the server will be progressively hammered by many requests, which
will clutter the logs, and make it more difficult to gracefully recover.

With this commit, we simply exponentially increase the delay each time, and add
a little jitter to give a better distribution.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[FIX] web: add exponential backoff strategy for lost connection
Before this commit, the web client had a naive strategy to handle lost
connections: it tried to poll the server every 2 seconds until a rpc
succeeds.

This works quite well from the perspective of the user, but may be a problem
from the perspective of the server.  If a server is down for a longish period,
then each users active tabs will then perform a request every 2 seconds. This
means that the server will be progressively hammered by many requests, which
will clutter the logs, and make it more difficult to gracefully recover.

With this commit, we simply exponentially increase the delay each time, and add
a little jitter to give a better distribution.

@robodoo robodoo added the seen 🙂 label Jan 11, 2019

@C3POdoo C3POdoo added the RD label Jan 11, 2019

@ged-odoo ged-odoo requested review from aab-odoo and odony Jan 11, 2019

@aab-odoo
Copy link
Contributor

aab-odoo left a comment

Seems good

@pedrobaeza

This comment has been minimized.

Copy link
Contributor

pedrobaeza commented Jan 11, 2019

Shouldn't a strategy like the Gmail one be more suitable?: it puts that there's a connection problem and retry in X seconds/minutes, with a button to retry now. With this, user is aware of the problem, and without interaction, it doesn't hammer the server, but you can interact for forcing the retry if needed.

@robodoo robodoo added the CI 🤖 label Jan 11, 2019

@ged-odoo

This comment has been minimized.

Copy link
Contributor

ged-odoo commented Jan 11, 2019

@pedrobaeza Yes, it would be nice to do that, but it is actually not doable with a moderate amount of work. Currently whenever some code tries to perform a rpc and it is rejected, then the failure will cause everything depending on it to be stopped. I do not see how to generically restore them.

I suppose that gmail can do that because it has a more well defined job.

Note that this commit does not change the fact that there is still a notification displayed with the connection lost message, and the user is still free to refresh the browser.

@pedrobaeza

This comment has been minimized.

Copy link
Contributor

pedrobaeza commented Jan 11, 2019

OK, I see. Thanks for the insights.

@ged-odoo

This comment has been minimized.

Copy link
Contributor

ged-odoo commented Jan 14, 2019

robodoo r+

@robodoo robodoo added the r+ 👌 label Jan 14, 2019

robodoo pushed a commit that referenced this pull request Jan 14, 2019

[FIX] web: add exponential backoff strategy for lost connection
Before this commit, the web client had a naive strategy to handle lost
connections: it tried to poll the server every 2 seconds until a rpc
succeeds.

This works quite well from the perspective of the user, but may be a problem
from the perspective of the server.  If a server is down for a longish period,
then each users active tabs will then perform a request every 2 seconds. This
means that the server will be progressively hammered by many requests, which
will clutter the logs, and make it more difficult to gracefully recover.

With this commit, we simply exponentially increase the delay each time, and add
a little jitter to give a better distribution.

closes #30136
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Jan 14, 2019

Merged, thanks!

@robodoo robodoo closed this Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment