remove sync response cache #380
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Benjamin-L
force-pushed
the
remove-sync-cache
branch
from
4ab65a4
to
2b47403
Compare
This cache can serve invalid responses, and has an extremely low hit rate. It serves invalid responses because because it's only keyed off the `since` parameter, but many of the other request parameters also affect the response or it's side effects. This will become worse once we implement filtering, because there will be a wider space of parameters with different responses. This problem is fixable, but not worth it because of the low hit rate. The low hit rate is because normal clients will always issue the next sync request with `since` set to the `prev_batch` value of the previous response. The only time we expect to see multiple requests with the same `since` is when the response is empty, but we don't cache empty responses. This was confirmed experimentally by logging cache hits and misses over 15 minutes with a wide variety of clients. This test was run on matrix.computer.surgery, which has only a few active users, but a large volume of sync traffic from many rooms. Over the test period, we had 3 hits and 5309 misses. All hits occurred in the first minute, so I suspect that they had something to do with client recovery from an offline state. The clients that were connected during the test are: - element web - schildichat web - iamb - gomuks - nheko - fractal - fluffychat web - fluffychat android - cinny web - element android - element X android Fixes: girlbossceo#336
Benjamin-L
force-pushed
the
remove-sync-cache
branch
from
85f37a1
to
3de3d8e
Compare
|
CI is now fixed on the main branch, so rebased and removed the lockfile commit. |
girlbossceo
approved these changes
May 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This cache can serve invalid responses, and has an extremely low hit rate.
It serves invalid responses because because it's only keyed off the
sinceparameter, but many of the other request parameters also affect the response or it's side effects. This will become worse once we implement filtering, because there will be a wider space of parameters with different responses. This problem is fixable, but not worth it because of the low hit rate.The low hit rate is because normal clients will always issue the next sync request with
sinceset to theprev_batchvalue of the previous response. The only time we expect to see multiple requests with the samesinceis when the response is empty, but we don't cache empty responses.This was confirmed experimentally by logging cache hits and misses over 15 minutes with a wide variety of clients. This test was run on matrix.computer.surgery, which has only a few active users, but a large volume of sync traffic from many rooms. Over the test period, we had 3 hits and 5309 misses. All hits occurred in the first minute, so I suspect that they had something to do with client recovery from an offline state. The clients that were connected during the test are:
Fixes: #336