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

Fix for timer throttling #698

Merged
merged 11 commits into from
Jun 8, 2021
Merged

Conversation

vishalnarkhede
Copy link
Contributor

@vishalnarkhede vishalnarkhede commented Jun 3, 2021

CLA

  • I have signed the Stream CLA (required).
  • Code changes are tested

Description of the changes, What, Why and How?

Latest chrome version came with some intensive timer throttling, which is causing issues with our websocket health check logic (ping-pong). SignalR from microsoft had the same issue as well. https://bugs.chromium.org/p/chromium/issues/detail?id=1186569

If you open codepen and keep in background tab, connection disconnects/reconnects quite frequently, because our “ping” in within setInterval (as shown in screenshot below)

Changelog

  • Switching from setInterval to adding setTimeout from onmessage. So everytime you receive a health-check event on websocket, you schedule a next ping. This is basically to avoid chained timers, which get throttled by chrome - https://developer.chrome.com/blog/timer-throttling-in-chrome-88/#chained-timers
  • While scheduling a next ping, also set a ticking time bomb (setTimeout) which will explode after pingInterval + 10000 ms and reconnect to websocket. This bomb should be defused (clearTimeout) everytime you receive health.check event.
  • Updated ping interval from 30 seconds to 25 seconds, to give some more breathing room for health check logic. If backend doesn't receive health.check event from client within 35 seconds, it shuts it down. Checked with backend team about this, this won't affect any traffic overload for backend socket

Testing

If you would like to test this issue

  • In your chrome browser, go to chrome://flags
  • Search for "Throttle Javascript timers in background.", and set its value to "Enabled 10 seconds"
  • Open react example chat app in chrome, and put the tab on background.
  • If you are on master branch of js client, you will see bunch of reconnects to websocket in about 5 minutes.
  • And if you switch to branch vishal/timer-throttling-fix on js client, you won't see any reconnects!!

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2021

Size Change: -170 B (0%)

Total Size: 228 kB

Filename Size Change
dist/browser.es.js 49.6 kB -49 B (0%)
dist/browser.full-bundle.min.js 28.5 kB +26 B (0%)
dist/browser.js 50.2 kB -47 B (0%)
dist/index.es.js 49.6 kB -52 B (0%)
dist/index.js 50.2 kB -48 B (0%)

compressed-size-action

@vishalnarkhede vishalnarkhede changed the title Fix for timer throttling WIP: Fix for timer throttling Jun 3, 2021
@vishalnarkhede vishalnarkhede changed the title WIP: Fix for timer throttling Fix for timer throttling Jun 7, 2021
@vishalnarkhede vishalnarkhede marked this pull request as ready for review June 7, 2021 10:35
mahboubii
mahboubii previously approved these changes Jun 7, 2021
src/connection.ts Outdated Show resolved Hide resolved
Co-authored-by: Amin Mahboubi <amin@getstream.io>
@vishalnarkhede vishalnarkhede merged commit 8f1ac69 into master Jun 8, 2021
@vishalnarkhede vishalnarkhede deleted the vishal/timer-throttling-fix branch June 8, 2021 14:44
jehartzog pushed a commit to ourfabriq/stream-chat-js that referenced this pull request Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants