-
Notifications
You must be signed in to change notification settings - Fork 270
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
feat(crypto) Provide a method to check whether server backup exists without hitting the server every time #4356
Conversation
6f1588f
to
944a1ba
Compare
eacb73b
to
fc8d59d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4356 +/- ##
==========================================
- Coverage 85.20% 85.19% -0.02%
==========================================
Files 281 281
Lines 30926 30963 +37
==========================================
+ Hits 26352 26379 +27
- Misses 4574 4584 +10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three things that are hard in computer science: invalidating caches, and off by ones. A few questions/suggestions below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Slightly concerned that the cached value may be outdated because another client erased the backup, but if you're fine with it, I'm fine with it :) (Isn't there a way to listen to some sync or to-device events, and realize the backup has been removed elsewhere?)
7c7d427
to
f3926ca
Compare
…ithout hitting the server every time
f3926ca
to
7b5bd0e
Compare
888b959
to
c37b485
Compare
Part of element-hq/element-meta#2638
Provide(Updated 2024-12-13 to reflect final implementation.)fast_exists_on_server
so we can call it when figuring out whether a UTD is expected.Modify
exists_on_server
to cache the last value we received. Providefetch_exists_on_server
that explicitly makes a server request to get the latest information.exists_on_server
does not always provide a fully-correct answer, but it is a best effort that is much cheaper than the fullfetch_exists_on_server
which always makes a request to the server. Since we only plan to call the new method when reporting UTDs, for now, the negative consequence of an incorrect answer is only to report the wrong kind of UTD.If you have suggestions for better ways to implement this, I am interested. I wish we didn't need to have a lock on the cached value but I couldn't see a better way.