Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer local updates if there are missing updates and only call GetMissingOnyxMessages once #38997

Merged
Changes from 11 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d32cd8b
defer updates if missing updates are fetched
chrispader Mar 26, 2024
e41558e
simplify
chrispader Mar 26, 2024
931527c
move deferred updates out of callback
chrispader Mar 26, 2024
5088441
simplify comments
chrispader Mar 26, 2024
9954e40
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Mar 26, 2024
a314f1f
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Mar 26, 2024
9d818d3
fix: apply deferred updates in order
chrispader Mar 26, 2024
f21ea78
simplify and improve applying deferred updates
chrispader Mar 27, 2024
3cc017e
check if deferred updates are older than the currently applied one
chrispader Mar 27, 2024
ce9027c
improve code
chrispader Mar 27, 2024
9889206
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Mar 27, 2024
c3034e0
fix: refetching logic
chrispader Mar 29, 2024
b9afdf3
fix: comments and logic
chrispader Mar 29, 2024
6a5c922
restructure file
chrispader Apr 1, 2024
4f35d1e
fix: re-write to detect possible multiple gaps in the deferred updates
chrispader Apr 1, 2024
095f09e
simplified conditions
chrispader Apr 1, 2024
6c748ac
further improve comments
chrispader Apr 1, 2024
b5deb28
fix: minor problems
chrispader Apr 2, 2024
2d08786
remove unnecessary code
chrispader Apr 2, 2024
25c8623
improve comment
chrispader Apr 2, 2024
7769810
fix: check for queryPromise
chrispader Apr 2, 2024
47613cf
update comment
chrispader Apr 2, 2024
fb9ac39
update comment
chrispader Apr 2, 2024
b9692d1
move lines
chrispader Apr 2, 2024
af920cd
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Apr 2, 2024
2514772
fix: comments
chrispader Apr 2, 2024
dab2b9f
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Apr 2, 2024
c0fe364
fix: wrong early return
chrispader Apr 2, 2024
252f0ab
fix: missing return
chrispader Apr 2, 2024
c1ca79b
Revert "fix: missing return"
chrispader Apr 2, 2024
67f322e
Update OnyxUpdateManager.ts
chrispader Apr 2, 2024
cd48514
Merge branch 'main' into @chrispader/prevent-simultaneous-calls-to-Ge…
chrispader Apr 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 90 additions & 4 deletions src/libs/actions/OnyxUpdateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Log from '@libs/Log';
import * as SequentialQueue from '@libs/Network/SequentialQueue';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {OnyxUpdatesFromServer, Response} from '@src/types/onyx';
import * as App from './App';
import * as OnyxUpdates from './OnyxUpdates';

Expand All @@ -27,6 +28,9 @@ Onyx.connect({
callback: (value) => (lastUpdateIDAppliedToClient = value),
});

let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined;
let deferredUpdates: Record<number, OnyxUpdatesFromServer> = {};

export default () => {
console.debug('[OnyxUpdateManager] Listening for updates from the server');
Onyx.connect({
Expand Down Expand Up @@ -66,32 +70,114 @@ export default () => {
// applied in their correct and specific order. If this queue was not paused, then there would be a lot of
// onyx data being applied while we are fetching the missing updates and that would put them all out of order.
SequentialQueue.pause();
let canUnpauseQueuePromise;
let canUnpauseQueuePromise: Promise<Response | void>;

// The flow below is setting the promise to a reconnect app to address flow (1) explained above.
if (!lastUpdateIDAppliedToClient) {
Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process');

deferredUpdateBeforeReconnect = updateParams;

// Since this is a full reconnectApp, we'll not apply the updates we received - those will come in the reconnect app request.
canUnpauseQueuePromise = App.finalReconnectAppAfterActivatingReliableUpdates();
} else {
// The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above.

// Get the number of deferred updates before adding the new one
const existingDeferredUpdatesCount = Object.keys(deferredUpdates).length;
// Add the new update to the deferred updates
chrispader marked this conversation as resolved.
Show resolved Hide resolved
deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams;

// If there are already deferred updates, then we don't need to fetch the missing updates again
if (existingDeferredUpdatesCount > 0) {
return;
}

console.debug(`[OnyxUpdateManager] Client is behind the server by ${Number(previousUpdateIDFromServer) - lastUpdateIDAppliedToClient} so fetching incremental updates`);
Log.info('Gap detected in update IDs from server so fetching incremental updates', true, {
lastUpdateIDFromServer,
previousUpdateIDFromServer,
lastUpdateIDAppliedToClient,
});

// Get the missing Onyx updates from the server
canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer);
}

canUnpauseQueuePromise.finally(() => {
OnyxUpdates.apply(updateParams).finally(() => {
// This function will apply the deferred updates in order after the missing updates are fetched and applied
function applyDeferredUpdates() {
// It should not be possible for lastUpdateIDAppliedToClient to be null, after the missing updates have been applied, therefore we don't need to handle this case
if (!lastUpdateIDAppliedToClient) {
return;
}

// If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp
if (deferredUpdateBeforeReconnect) {
OnyxUpdates.apply(deferredUpdateBeforeReconnect);
deferredUpdateBeforeReconnect = undefined;
}

// We only want to apply deferred updates that are newer than the last update that was applied to the client.
// At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out.
const pendingDeferredUpdates = Object.fromEntries(
Object.entries(deferredUpdates).filter(([lastUpdateID]) => {
// Same as above, it should not be possible for lastUpdateIDAppliedToClient to be null.
// Still, if so we want to keep the deferred update in the list
if (!lastUpdateIDAppliedToClient) {
return true;
}
return (Number(lastUpdateID) ?? 0) > lastUpdateIDAppliedToClient;
}),
);
const pendingDeferredUpdateValues = Object.values(pendingDeferredUpdates);

function unpauseQueueAndReset() {
console.debug('[OnyxUpdateManager] Done applying all updates');
Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
SequentialQueue.unpause();
}

// If there are no remaining deferred updates after filtering out outdated ones,
// we can just unpause the queue and return
if (pendingDeferredUpdateValues.length === 0) {
unpauseQueueAndReset();
return;
chrispader marked this conversation as resolved.
Show resolved Hide resolved
}

let lastValidDeferredUpdateID = 0;
// In order for the deferred updates to be applied correctly in order,
// we need to check if there are any gaps deferred updates.
function areDeferredUpdatesChained(): boolean {
let isFirst = true;
for (const update of pendingDeferredUpdateValues) {
// If it's the first one, we need to skip it since we won't have the previousUpdateID
// if the previousUpdateID key exists, then we can just move on since there's no gap
if (isFirst || pendingDeferredUpdates[Number(update.previousUpdateID)]) {
lastValidDeferredUpdateID = Number(update.lastUpdateID);
isFirst = false;
// eslint-disable-next-line no-continue
continue;
}

return false;
}

return true;
}

// If we detect a gap in the deferred updates, re-fetch the missing updates.
if (!areDeferredUpdatesChained()) {
canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastValidDeferredUpdateID).finally(applyDeferredUpdates);
chrispader marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Promise.all(pendingDeferredUpdateValues.map((update) => OnyxUpdates.apply(update))).finally(() => {
unpauseQueueAndReset();
deferredUpdates = [];
});
});
}

canUnpauseQueuePromise.finally(applyDeferredUpdates);
},
});
};
Loading