[lib] Stop checking for entry store inconsistencies
AcceptedPublic
Actions

Authored by tomek on Wed, May 22, 5:55 AM.

Details

Reviewers

kamil
marcin

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 created this revision.Wed, May 22, 5:55 AM

Herald added a subscriber: ashoat. · View Herald TranscriptWed, May 22, 5:55 AM

Harbormaster completed remote builds in B29163: Diff 40534.Wed, May 22, 6:11 AM

tomek requested review of this revision.Wed, May 22, 6:11 AM

marcin accepted this revision.Wed, May 22, 7:18 AM

marcin edited the test plan for this revision. (Show Details)

This revision is now accepted and ready to land.Wed, May 22, 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.

In D12177#347399, @ashoat wrote:

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.

Thanks for explaining!