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

Sync jobs time out all too often #837

Open
KitsuneRal opened this issue Nov 27, 2024 · 1 comment · May be fixed by #838
Open

Sync jobs time out all too often #837

KitsuneRal opened this issue Nov 27, 2024 · 1 comment · May be fixed by #838
Assignees
Labels
regression Something broken since the last release

Comments

@KitsuneRal
Copy link
Member

KitsuneRal commented Nov 27, 2024

Describe the bug
It can be seen in logs and client UIs that syncing frequently times out. This is most likely because of the smaller timeout values set in f26e3ed to address unit tests occasionally failing when the first request gets stuck for some reason. Long polling implies that sync requests may stay with the server for quite some time, and the current 30 seconds are definitely not long enough to support that.

Expected behavior
It's probably best to revert timeouts to their original values and adjust timeouts in unit tests instead. They might run longer but network requests getting stuck are not a normal situation anyway.

@KitsuneRal KitsuneRal added the regression Something broken since the last release label Nov 27, 2024
@KitsuneRal KitsuneRal moved this to In work in libQuotient 1 Nov 27, 2024
@KitsuneRal KitsuneRal self-assigned this Nov 27, 2024
@KitsuneRal
Copy link
Member Author

Fun fact: the default timeout for Connection::syncLoop() is also 30 seconds, but it's the server-side time, so the library might be giving up on waiting just before the empty response arrives, due to the network latency. Merely increasing the default timeout to 40s could remove those failures.

KitsuneRal added a commit that referenced this issue Nov 27, 2024
SyncJob doesn't only have unlimited retries now, it also backs off in a
different way, to address for its special situation as a de-factor keep-
alive signal. Fixes #837.
KitsuneRal added a commit that referenced this issue Nov 27, 2024
SyncJob doesn't only have unlimited retries now, it also backs off in a
different way, to address for its special situation as a de-factor keep-
alive signal. Fixes #837.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Something broken since the last release
Projects
Status: In work
Development

Successfully merging a pull request may close this issue.

1 participant