keyserver] avoid out of sync `updatesCurrentAsOf` between client and keyserver
ClosedPublic
Actions

Authored by kamil on Nov 28 2023, 7:39 AM.

Details

Reviewers

inka
tomek
michal

Commits

rCOMM06cb756f0bae: [web/lib/keyserver] avoid out of sync `updatesCurrentAsOf` between client and…

Summary

This is fix to issue described in: ENG-5906.

In D9948 I started to persist updatesCurrentAsOf.
I also updated setInitialReduxState action to rely on rehydrated data when it's a staff user.
What I missed is updating currentAsOfPromise, which for updatesCurrentAsOf was using server time here.
This means that after reloading the app, the client (staff) has its own updatesCurrentAsOf from the previously received update (previous session) while the keyserver has newer updatesCurrentAsOf created while returning the initial state version.
This causes this condition to be true, and as a result was causing FULL_STATE_SYNC here which on prod kills the socket because of the amount of data to transfer.

This diff should make updatesCurrentAsOf synced between client and server.

This also fixes issue described here. Previously, when updatesCurrentAsOf was just started to be persisted, it gets rehydrated with undefined/0 value, this means after loading app all updates were started to be fetched. Right now if updatesCurrentAsOf is 0 we fetch everything in InitialReduxStateResponder and set updatesCurrentAsOf as serverTime.

This is only for staff, for standard users everything should work as previously.

Test Plan

Reload app and make sure that there is no FULL_STATE_SYNC.
Make sure updates work properly.
Make sure updatesCurrentAsOf has correct value.
Make sure policies logic (which is related to updatesCurrentAsOf value) works.

Diff Detail

Repository

rCOMM Comm

Branch

land-fix-prod-again

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Nov 28 2023, 7:39 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptNov 28 2023, 7:39 AM

kamil edited the summary of this revision. (Show Details)Nov 28 2023, 7:41 AM

fix condition

kamil edited the summary of this revision. (Show Details)Nov 28 2023, 7:57 AM

kamil published this revision for review.Nov 28 2023, 8:02 AM

kamil added inline comments.

keyserver/src/responders/redux-state-responders.js
144–147 ↗	(On Diff #33946)	this might cause returning part of the data twice, here in responder and then via updates, but it was discussed in ENG-5316
web/redux/redux-setup.js
129–135 ↗	(On Diff #33946)	this can be removed, now in `setInitialReduxState` payload we get the proper value for both staff/non-staff users, which is persisted `updatesCurrentAsOf` on client or server time
web/types/redux-types.js
52 ↗	(On Diff #33946)	same as in D9949, it's safe to modify this

Harbormaster completed remote builds in B24570: Diff 33946.Nov 28 2023, 8:29 AM

Harbormaster completed remote builds in B24572: Diff 33948.Nov 28 2023, 9:00 AM

Doesn't this result in updates being sent for data that was already sent in initial redux state?

web/redux/initial-state-gate.js
46–48 ↗	(On Diff #33948)	`getInitialReduxState` is already refactored for multiple keyservers. You have to select all `updatesCurrentAsOf`. You probably want to write a selector similar to `cookiesSelector`. Then in `getInitialReduxState` you have to use the correct `updatesCurrentAsOf` in every request