Page MenuHomePhabricator

[lib] Stop checking for entry store inconsistencies
ClosedPublic

Authored by tomek on May 22 2024, 5:55 AM.
Tags
None
Referenced Files
F3375101: D12177.id40534.diff
Tue, Nov 26, 6:17 PM
Unknown Object (File)
Sat, Nov 23, 3:30 AM
Unknown Object (File)
Fri, Nov 22, 11:26 PM
Unknown Object (File)
Sat, Nov 16, 7:44 AM
Unknown Object (File)
Sun, Nov 10, 1:14 PM
Unknown Object (File)
Sun, Nov 10, 8:38 AM
Unknown Object (File)
Sun, Nov 10, 7:51 AM
Unknown Object (File)
Sun, Nov 10, 4:22 AM
Subscribers

Details

Summary

We aren't persisting them in Redux at this point, which means that this assertion was pointless.

Depends on D12170

https://linear.app/comm/issue/ENG-3486/move-entrystore-to-sqlite

Test Plan

Close and reopen the app a couple of times. Prior to this diff we would see alert each time. With this diff the alert disappeared.

Diff Detail

Repository
rCOMM Comm
Branch
entries
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested review of this revision.May 22 2024, 6:11 AM
marcin edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.May 22 2024, 7:19 AM

Can you explain a bit more? Are we completely removing the logic for finding EntryStore inconsistencies? If so, I'd like to understand why that's necessary. We moved threads (for instance) to SQLite and still have inconsistency reports for those.

Can you explain a bit more? Are we completely removing the logic for finding EntryStore inconsistencies? If so, I'd like to understand why that's necessary. We moved threads (for instance) to SQLite and still have inconsistency reports for those.

I took a look at the code and confirmed with @kamil that we don't have the SQLite inconsistencies checks for the thread store. If we don't persist a store using redux-persist, it's not possible to compare the SQLite data with it.

I think the word inconsistency was unfortunate here because we have a different type of inconsistency check for the thread store - we're checking if the state on the keyserver matches the state of the client using the state check mechanism. This type of inconsistency check will be kept for the entry store as well (line 687).

Ah, I see – sorry for the confusion.

Interesting that we already have assertEntryStoresAreEqual in the codebase on master, but we only store the entries in redux-persist. Is it not currently used on master?

Separately, do we want to consider leaving the data in both redux-persist and SQLite for some time, to make sure there aren't any issues/inconsistencies between the two?

Ah I see, it was introduced in D12147. Do we plan to defer landing this (and the prior diff that moves from using redux-persist to SQLite as the ground truth) for a couple weeks

Currently, the data is stored in both SQLite and redux-persist. We use the data from SQLite only when checking for inconsistencies - the state in Redux is always based on redux-persist. The plan is to keep this approach for a couple of weeks and then start using SQLite as the source of truth - this is handled by the last three diffs in this stack and tracked in https://linear.app/comm/issue/ENG-8234/finish-migrating-entry-store-to-sqlite.