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

Increase BUFFER_PERIOD_MS from 80s to 20mins #390

Closed
wants to merge 1 commit into from

Conversation

lukebarnard1
Copy link
Contributor

@lukebarnard1 lukebarnard1 commented Mar 15, 2017

Fixes element-hq/element-web#2737

Seems to work - I forced Chome into a heavily throttled state during /sync (1-10kb/s).
2017-03-15-135556_427x45_scrot

@richvdh
Copy link
Member

richvdh commented Mar 15, 2017

I'm not sure that we can just bump this to 20 minutes without adding some sort of indicator to the UI that something is going on. It looks to me as though the 20 minute figure was pulled out of the air as an arbitrary value which effectively disables the timeout; I'd like to see a better justification.

How does this look in the UI (a) on first connection, and (b) when reconnecting after an error?

@richvdh richvdh assigned lukebarnard1 and unassigned richvdh Mar 15, 2017
@lukebarnard1
Copy link
Contributor Author

lukebarnard1 commented Mar 15, 2017

How does this look in the UI (a) on first connection, and (b) when reconnecting after an error?

It just looks like it's loading the entire time, i.e. a spinner spins for ages.

We could show some sort of progress bar based on /sync. Or some indication from the status bar at the top of page like "Disconnected... Reconnecting... Connected" a la facebook messenger.

the 20 minute figure was pulled out of the air as an arbitrary value

Yes, it was.

effectively disables the timeout

Yes, it does, effectively removing one layer of retrying, which seems a bit pointless. But with a limit of 80s, anything longer is assumed failure. I suppose it's a question of how long the user is willing to wait for /sync and I'm guessing this is much less than 20mins in the general case.

Perhaps it would be better to have some option that gets set in the clients and given to the js-sdk that disables the timeout. And we could call it "poor connection mode" or something similar?

This has the disadvantage that the user has to worry about how fast their connection is, which is rubbish.

Thinking about it, I wonder if a simple timeout doesn't really suit this. If there's some progress in a very slow sync, that's still progress and we shouldn't reconnect because of it.

@t3chguy t3chguy deleted the luke/increase-sync-timeout branch August 11, 2022 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants