python-zulip-api icon indicating copy to clipboard operation
python-zulip-api copied to clipboard

api: Use exponential backoff in call_on_each_event.

Open neiljp opened this issue 4 years ago • 4 comments

A long-term TODO here has been to move from a 1s pause after each failure towards an exponential backoff approach, as proposed here.

@timabbott See last_event_id handling topic on #zulip-terminal from last August.

I'm unsure of the best parameter set to use here; I've taken values from other uses of the backoff and filled the default values in explicitly to aid in reasoning.

I'm not sure if we want to switch to backoff.keep_going but rather stay with the previous while True approach, since this is essentially an unending event loop.

neiljp avatar Apr 18 '20 05:04 neiljp

@timabbott I've added some followup commits which first do backoff for the do_register as well, and then inline it like you mentioned, which I think is cleaner. I'm not sure whether we want the intermediate refactoring or not.

There's also a 1s sleep in the error_retry in do_api_query, but I'm not sure if we want that to have backoff too?

neiljp avatar Apr 26 '20 10:04 neiljp

There's also a 1s sleep in the error_retry in do_api_query, but I'm not sure if we want that to have backoff too?

I think we certainly should. If we start to fall over due to overload and start returning 5xx, I don't think we want every client switching from 1 request every 10s (longpoll) to every second -- which is what the current logic does.

alexmv avatar Aug 01 '20 00:08 alexmv

@neiljp do you have time to update this? With #611 I'd like to get this resolve and then do a release.

timabbott avatar Aug 10 '20 23:08 timabbott

Heads up @neiljp, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Aug 24 '21 19:08 zulipbot