Skip to content

Conversation

alexmv
Copy link
Contributor

@alexmv alexmv commented Aug 1, 2020

This reduces the number of retries that might spam APIs.

There is some complexity here which is left un-managed -- for
instance, maybe_restart_mirroring_script does a number of restart
attempts, and then fails, but will be retried every 15s by the
surrounding process_loop. Previously, it would merely have looped
forever inside maybe_restart_mirroring_script.

Three loops are intentionally left as infinite while True loops,
that merely cap their backoff at the default 90s. Their callers do
not expect, or have any way to handle more gracefully, a failure of
the expected-infinite-loop in process_loop or zulip_to_zephyr.
They maintain their previous behavior of retrying forever, albeit more
slowly.

@alexmv
Copy link
Contributor Author

alexmv commented Aug 1, 2020

See also #586.

@timabbott
Copy link
Member

Posted some comments; otherwise lgtm.

alexmv added 3 commits August 3, 2020 12:47
This reduces the number of retries that might spam APIs.

There is some complexity here which is left un-managed -- for
instance, maybe_restart_mirroring_script does a number of restart
attempts, and then fails, but will be retried every 15s by the
surrounding `process_loop`.  Previously, it would merely have looped
forever inside maybe_restart_mirroring_script.

Three loops are intentionally left as infinite `while True` loops,
that merely cap their backoff at the default 90s.  Their callers do
not expect, or have any way to handle more gracefully, a failure of
the expected-infinite-loop in `process_loop` or `zulip_to_zephyr`.
They maintain their previous behavior of retrying forever, albeit more
slowly.
@zulipbot zulipbot added size: L and removed size: M labels Aug 3, 2020
@alexmv alexmv merged commit 9745ec9 into zulip:master Aug 10, 2020
@alexmv alexmv deleted the zephyr-backoff branch December 1, 2020 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants