listen_forever always waits the maximum amount of time between sync retries

[bjlaub]

See this line: https://github.com/matrix-org/matrix-python-sdk/blob/master/matrix_client/client.py#L375 which sets the value for bad_sync_timeout to 5000 seconds (probably intended to be milliseconds?), and this line: https://github.com/matrix-org/matrix-python-sdk/blob/master/matrix_client/client.py#L386 which calls time.sleep with that value. Note that time.sleep expects seconds, not millis.

This means that if _sync fails with an HTTP 500, the client will always wait self.bad_sync_timeout_max seconds (currently 1 hour) before retrying.

It looks like there's supposed to be some exponential backoff logic between retries, but because the initial timeout is set so high it always selects the ceiling of one hour.

[kurisuD]

shouldn't bad_sync_timeout at L375 be bad_sync_timeout_limit ?

[non-Jedi]

Probably related to #129 and #131, but I haven't dug into any of these issues deeply enough to say for sure. @bjlaub, would you mind updating your issue description with permalinks to the appropriate lines of code. Since master has changed since you filed this issue, hard to tell exactly what you were referring to.

I've had a look at the exponential backoff algorithm in listen_forever.

It's not as bad as it looks. The retry time will be 5000 seconds only if the first ever sync fails. If it keeps failing, it goes to retry every 3600 seconds (1 hour).

If it succeeds, the timeout resets back to 5 seconds. It will double each time it fails, up to the maximum of 3600, and resets back to 5 as soon as it succeeds again.

So all in all, it's not too bad, but it's not right either. I agree that it is most likely that 5 was meant instead of 5000 in

matrix-python-sdk/matrix_client/client.py

Line 424 in aa17a6a

bad_sync_timeout = 5000

@non-Jedi could you do the honours and edit this, or should I make a whole PR for this very simple change?

[non-Jedi]

Maybe you've got a better idea of the wider context. I'm scared to touch that line because I really don't get the logic. Why is bad_sync_timeout set to an entirely different value a few lines lower in the midst of the loop?

matrix-python-sdk/matrix_client/client.py

Line 429 in aa17a6a

bad_sync_timeout = 5

That line is inside a try block, directly after the sync attempt. So if the sync fails, an exception is thrown, and that line is never reached.

If the sync repeatedly does not succeed, it will keep skipping that line and keep doubling the delay as per the code in the except block.

If the sync succeeds at any time, that line is reached and the delay is reset to a low value of 5 seconds.

In short, the whole thing is doing exponential back-off.

Sync fails? Retry in 5 seconds. Still fail? Wait 10 seconds. Then 20, 40, 80, 160 and so on, up to one hour.