[web] start using threads from database
ClosedPublic
Actions

Authored by kamil on Nov 23 2023, 6:06 AM.

Details

Reviewers

tomek
michal

Commits

rCOMM65c3cc5b137b: [web] start using threads from database

Summary

Migrate threads to DB when needed
Use threads from DB and avoid fetching from keyserver when data is present.

Depends on D9956

Test Plan

Test plan was done twice, in Chrome and Safari:

Check if migrating works
Check if threads are not fetched from keyserver when there are in DB
Modify thread and reload app
Modify thread on different account
Close app, modify thread on different account and open app (test if checkpointing works)

I tested this on the account with ~4k threads, I didn't notice any perf downgrade, moreover, I think initial app load was faster when reading threads from DB.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Nov 23 2023, 6:06 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptNov 23 2023, 6:06 AM

Harbormaster completed remote builds in B24376: Diff 33569.Nov 23 2023, 6:26 AM

kamil published this revision for review.Nov 23 2023, 6:36 AM

ashoat added inline comments.Nov 23 2023, 7:28 AM

web/redux/initial-state-gate.js
33 ↗	(On Diff #33569)	I think your IDE added this

tomek accepted this revision.Nov 24 2023, 3:55 AM

tomek added inline comments.

keyserver/src/responders/redux-state-responders.js
95 ↗	(On Diff #33569)	Can we use `canUseDatabaseOnWeb` from https://phab.comm.dev/D9948?
161 ↗	(On Diff #33569)	I'm wondering if it would be beneficial to differentiate between empty thread infos and excluding them. So in this case, instead of returning an empty object, we would return e.g. `null`. Would that be useful?
web/redux/initial-state-gate.js
65 ↗	(On Diff #33569)	Should we check the length just like in https://phab.comm.dev/D9954?
72 ↗	(On Diff #33569)	Can we use `canUseDatabaseOnWeb`?

This revision is now accepted and ready to land.Nov 24 2023, 3:55 AM

kamil mentioned this in D9951: [web/lib] move reading and populating data to state gate.Nov 27 2023, 3:32 AM

kamil mentioned this in D9954: [web] processing ThreadStore ops on worker.Nov 27 2023, 5:31 AM

address review

keyserver/src/responders/redux-state-responders.js
161 ↗	(On Diff #33569)	I don't think that improves or changes anything, there is no advance in the current state.
web/redux/initial-state-gate.js
33 ↗	(On Diff #33569)	ahhh... IDE did it again 😠
65 ↗	(On Diff #33569)	This is the result of the function from D9954. So the check if inside `getClientStore`
72 ↗	(On Diff #33569)	sure, that was my idea but I messed something while creating diff