Closed radex closed 3 years ago
@techfort This PR should be ready to review if you've got time :)
@radex Thanks for this work. @techfort Do you plan to perform a npm release with this improvement?
(I use this adapter in my project: https://champagnethomas.medium.com/elevate-desktop-app-2021-update-9155fd74bb85)
Thanks !
@thomaschampagne Nice, cool to see that my LokiJS improvements aren't used only by WatermelonDB :) You can use @nozbe/lokijs NPM prereleases or make your own fork
Oh nice !!! I put that in my package.json right now !
1.5.12 published, thanks for the hard work guys
On Mon, 19 Apr 2021 at 21:24, Thomas Champagne @.***> wrote:
Oh nice !!! I put that in my package.json right now !
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/techfort/LokiJS/pull/869#issuecomment-822761013, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7QK7OBYCWJXIVXWGLV4LDTJSGQNANCNFSM4UKJHV7Q .
IncrementalIDBAdapter has had a flaw. Saving only changed chunks (instead of dumping the whole in-memory database into IDB) can easily lead to database corruption if there's more than one writer (multiple tabs of the same web app).
I did consider this possibility when developing #808, but thinking about how this is meant to be used with an app that synchronizes (sync should make the in-memory state of multiple tabs exactly the same, avoiding inconsistency, and therefore corruption), I naively concluded that this won't be a problem. In hindsight, that's obviously wrong, and caused all sorts of issues for Nozbe Teams users. I was quite easily able to find multiple reproducible paths for corrupting the loki database. Most corruptions are subtle (e.g. binary index being slightly out of order) and don't manifest themselves so easily, but sometimes you can get a broken app unable to launch, as there is an order of events where two writers will save a record with the same id in multiple chunks (leading to a UniqueIndex throwing an error). #856 probably made this worse, as collection metadata save can be skipped.
The fix is quite straightforward and I could not come up with a way to break it:
So the worst case scenario gets roughly the same performance as
LokiIndexedAdapter
, but most cases (one tab/no contention) have the same performance as before, though with slightly higher latency (we must fetch theloki
chunk at each save, but it adds almost nothing to the time where the main thread is blocked, just a delay waiting for the concurrent IndexedDB process to give us an answer).put
into IDB immediately, without an intermediate array (gets big on large databases)