-
Notifications
You must be signed in to change notification settings - Fork 450
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
⚡️ Remove server-side Presence transformations #538
Conversation
524b9d3
to
f9ef1ec
Compare
This change addresses a performance issue with Presence, whereby every single Presence update will cause a Database call to fetch the latest ops, and transform Presence up. This can be quite costly if there are a lot of Presence updates, and adversely impact the "normal" operation of ShareDB. This is particularly strange for the "happy case", where two clients are exchanging Presence on the same version of a document (where there should be no need to query the database at all for Presence). We already have mechanisms in the client to transform presence, as well as re-request stale presence, so we can remove the server-side Presence transformations all together. This alleviates pressure on the server, at the cost of decreased performance in edge cases, where clients need to re-request Presence from other clients, or catch up on cached ops. However, this should be a sensible change to make, since we're trying to optimise for the happy path, rather than edge cases. Note that this changes a test case: we have to resubscribe `doc1` in this test case, otherwise it's never up-to-date, and `presence2` continues to throw out its presence updates. This could happen in a real-world situation (where `doc1` stays offline), but it's arguable that this is uninteresting presence anyway, since that client is not "live".
f9ef1ec
to
febb54f
Compare
@alecgibson, I've run the tests. Now it's faster a bit, but still, there are slow cases of op applying. CPU usage of one of the sharedb pods. New slow.log BTW, I have throttling for sending presence not often than every second. |
@yaroslavputria any chance of more details on that CPU profile, like a flame chart? I can't really tell you what the issue is without more details, especially because your attached logs now have no mention of presence at all. Also can you please remind me of the parameters of your load testing? I think Presence in its current form has always been vulnerable to some fan-out effects; we can try to minimise them if you can give us more details, please! |
Very nice! |
Some other things that could also help: |
@alecgibson, @ericyhwang, thanks for the hints!
I'll try to get CPU profiling (but I think it will be on the next week).
30 users typing within 6 documents (5 users per document). |
@yaroslavputria could you please also try this by not moving the cursor immediately after the inserted character? Presence should already do all the heavy lifting for you by transforming ops on remote clients by the applied op, so you don't actually need to broadcast this information. Submitting presence at the same time as an op actually hits all sorts of fun presence edge cases, which is probably why you're seeing such a heavy CPU load. We transform presence client side to avoid the need for this sort of thing all together. You'll still need to set the presence at the start of the test, obviously, and if you want to flex other bits of presence, you could add random presence moves in between ops. |
@alecgibson, I've just seen your last comment. |
@yaroslavputria I've updated the Presence example in #539 Basically, you only ever need to update your Presence when the user "actively" moves their selection. If their selection update is the result of an op, there's no need to send the information. For example:
That being said, people might be moving around a document as other people are typing, so it's probably still worth trying different performance setups. A CPU flame chart would be super useful to debug where the performance bottlenecks are happening. |
@alecgibson, thanks for the clarifications. |
Closing this as no longer required. If we think this might help in future, we can reopen. |
Fixes share/sharedb-mongo#123
This change addresses a performance issue with Presence, whereby every
single Presence update will cause a Database call to fetch the latest
ops, and transform Presence up.
This can be quite costly if there are a lot of Presence updates, and
adversely impact the "normal" operation of ShareDB. This is particularly
strange for the "happy case", where two clients are exchanging Presence
on the same version of a document (where there should be no need to
query the database at all for Presence).
We already have mechanisms in the client to transform presence, as well
as re-request stale presence, so we can remove the server-side Presence
transformations all together.
This alleviates pressure on the server, at the cost of decreased
performance in edge cases, where clients need to re-request Presence
from other clients, or catch up on cached ops. However, this should be a
sensible change to make, since we're trying to optimise for the happy
path, rather than edge cases.
Note that this changes a test case: we have to resubscribe
doc1
inthis test case, otherwise it's never up-to-date, and
presence2
continues to throw out its presence updates. This could happen in a
real-world situation (where
doc1
stays offline), but it's arguablethat this is uninteresting presence anyway, since that client is not
"live".