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

Treat all signal messages as ping response #933

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Treat all signal messages as ping response #933

merged 2 commits into from
Nov 14, 2023

Conversation

davidzhao
Copy link
Member

In cases of rooms that are highly active, it's possible to have a lot of signal messages queued that it blocks the ping messages for awhile.

Since the purpose of ping/pong is to ensure signal channel is active, it makes sense to use any type of signal traffic as that indicator of liveliness.

In cases of rooms that are highly active, it's possible to have a lot of
signal messages queued that it blocks the ping messages for awhile.

Since the purpose of ping/pong is to ensure signal channel is active, it
makes sense to use any type of signal traffic as that indicator of liveliness.
Copy link

changeset-bot bot commented Nov 13, 2023

🦋 Changeset detected

Latest commit: 0a5cbdd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
livekit-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 13, 2023

size-limit report 📦

Path Size
dist/livekit-client.esm.mjs 77.16 KB (+0.02% 🔺)
dist/livekit-client.umd.js 82.69 KB (+0.02% 🔺)

@lukasIO
Copy link
Contributor

lukasIO commented Nov 13, 2023

I think what we're missing with this approach is detecting when only sending of signal messages is broken.
IIRC the initial motivation to implement ping/pong as is, was due to the fact that we had seen some odd cases where it looked like server updates were still coming through but signal messages sent from the client to the server seemingly weren't.

@davidzhao
Copy link
Member Author

I think what we're missing with this approach is detecting when only sending of signal messages is broken. IIRC the initial motivation to implement ping/pong as is, was due to the fact that we had seen some odd cases where it looked like server updates were still coming through but signal messages sent from the client to the server seemingly weren't.

interesting.. so websocket was broken only in one direction? that's certainly a strange behavior.. but I guess we've seen stranger!

The motivation for this PR is due to a customer report that after paging very quickly, they were disconnected from the room due to idle timer. The reason for that failure is ping timer unable to deliver its messages to the server quickly enough.. likely due to the number of subscription changes to send before then.

@davidzhao davidzhao merged commit 93ee41b into main Nov 14, 2023
2 checks passed
@davidzhao davidzhao deleted the dz-handle-ping branch November 14, 2023 08:08
@github-actions github-actions bot mentioned this pull request Nov 14, 2023
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