Identify the tree-ready signal by reference instead of counting

The ready signal was an incrementing counter, so its correctness rested on an
argument about scale rather than a property of the value. Overflow was not
reachable in a page session — nine quadrillion refreshes — and the failure mode
was fail-closed, leaving the phase open rather than reporting a load as healthy.
It was still a bound that had to be argued rather than one that could not be
exceeded.

The signal is now a fresh object published per load and compared by reference.
There is no value to exhaust or wrap, and the comparison is identity rather than
ordering, so a store reset cannot leave the consumer permanently ahead of the
producer either.

Related: IcM 865096261
This commit is contained in:
Dmitrii Shilov
2026-09-14 12:07:57 +01:00
parent 3b2efdb043
commit 94fe5648e5
4 changed files with 32 additions and 28 deletions
+5 -5
View File
@@ -447,12 +447,12 @@ export default class Explorer {
startKey,
);
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded);
// Start DatabaseTreeRendered before publishing the revision that triggers the render,
// so the phase is always open by the time ResourceTree acknowledges it. Bumping the
// revision is what tells the tree "this is the data the current load is expected to
// show"; the phase is completed only for a render carrying that revision.
// Start DatabaseTreeRendered before publishing the token that triggers the render, so the
// phase is always open by the time ResourceTree acknowledges it. Publishing the token is
// what tells the tree "this is the data the current load is expected to show"; the phase
// is completed only for a render carrying that token.
scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered);
useDatabases.setState({ treeReadyRevision: useDatabases.getState().treeReadyRevision + 1 });
useDatabases.setState({ treeReadyToken: {} });
} catch (error) {
TelemetryProcessor.traceFailure(
Action.LoadCollections,
+6 -5
View File
@@ -13,11 +13,12 @@ interface DatabasesState {
resourceTokenCollection: ViewModels.CollectionBase;
sampleDataResourceTokenCollection: ViewModels.CollectionBase;
databasesFetchedSuccessfully: boolean; // Track if last database fetch was successful
// Incremented once a load has produced the data the resource tree is expected to show.
// Republished each time a load has produced the data the resource tree is expected to show.
// The DatabaseTreeRendered phase is only completed for a render that carries the current
// revision, so an earlier render (databases fetched, collections still loading) cannot be
// mistaken for the loaded tree. 0 means no load has reached that point yet.
treeReadyRevision: number;
// token, so an earlier render (databases fetched, collections still loading) cannot be
// mistaken for the loaded tree. Compared by reference rather than counted, so there is no
// value to overflow or wrap; undefined means no load has reached that point yet.
treeReadyToken: object | undefined;
searchText: string;
sortOrder: DatabaseSortOrder;
pinnedDatabaseIds: Set<string>;
@@ -57,7 +58,7 @@ export const useDatabases: UseStore<DatabasesState> = create((set, get) => ({
resourceTokenCollection: undefined,
sampleDataResourceTokenCollection: undefined,
databasesFetchedSuccessfully: false,
treeReadyRevision: 0,
treeReadyToken: undefined,
searchText: "",
sortOrder: loadSortOrder(),
pinnedDatabaseIds: loadPinnedDatabases(),
@@ -8,7 +8,7 @@
* (ResourceTree.tsx), every completion arrived before the phase existed and was dropped:
* missing in 1624 of 1627 DatabaseLoad timeouts over three days.
*
* The fix is on the producer side — a ready revision published once the load has produced the
* The fix is on the producer side — a ready token published once the load has produced the
* data the tree is expected to show, acknowledged by the render that carries it. These tests
* pin the monitor half of that contract: completions for phases that were never opened are
* refused rather than backdated, so a render observed too early cannot stand in for the
@@ -45,7 +45,7 @@ const loadUpToTreeReady = () => {
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabasesFetched);
scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded);
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.CollectionsLoaded);
// Explorer publishes the ready revision immediately after opening the phase.
// Explorer publishes the ready token immediately after opening the phase.
scenarioMonitor.startPhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered);
};
@@ -62,9 +62,9 @@ describe("DatabaseLoad phase accounting (IcM 865096261)", () => {
jest.useRealTimers();
});
it("completes healthy once the render carrying the ready revision is acknowledged", () => {
it("completes healthy once the render carrying the ready token is acknowledged", () => {
loadUpToTreeReady();
// ResourceTree commits the revision-bearing render and acknowledges it.
// ResourceTree commits the token-bearing render and acknowledges it.
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered);
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, CommonMetricPhase.Interactive);
+17 -14
View File
@@ -43,31 +43,34 @@ export function useInteractive(scenario: MetricScenario, enabled = true) {
/**
* Hook to manage DatabaseLoad scenario phase completions.
*
* DatabaseTreeRendered is completed only for a render that carries the ready revision
* published by the current load (Explorer.refreshAndExpandNewDatabases). An earlier render —
* databases fetched but collections still loading — carries a stale revision and is ignored,
* so it cannot be mistaken for the loaded tree. Each revision is acknowledged at most once,
* which also discards stale callbacks from a superseded load.
* DatabaseTreeRendered is completed only for a render that carries the ready token published
* by the current load (Explorer.refreshAndExpandNewDatabases). An earlier render — databases
* fetched but collections still loading — carries the previous token and is ignored, so it
* cannot be mistaken for the loaded tree. Each token is acknowledged at most once, which also
* discards stale callbacks from a superseded load.
*
* An account with no databases still publishes a revision and renders an empty tree, so it
* The token is a fresh object compared by reference, so repeated refreshes cannot exhaust or
* wrap it the way an incrementing counter could.
*
* An account with no databases still publishes a token and renders an empty tree, so it
* completes normally rather than stalling.
*/
export function useDatabaseLoadScenario(databaseTreeNodes: unknown[], fetchSucceeded: boolean) {
const treeReadyRevision = useDatabases((state) => state.treeReadyRevision);
const acknowledgedRevision = React.useRef(0);
const treeReadyToken = useDatabases((state) => state.treeReadyToken);
const acknowledgedToken = React.useRef<object | undefined>(undefined);
// Track DatabaseTreeRendered phase. Runs after commit, so the tree carrying this revision
// is on screen by the time the phase is completed.
// Track DatabaseTreeRendered phase. Runs after commit, so the tree carrying this token is on
// screen by the time the phase is completed.
React.useEffect(() => {
if (!fetchSucceeded || treeReadyRevision === 0) {
if (!fetchSucceeded || !treeReadyToken) {
return;
}
if (acknowledgedRevision.current >= treeReadyRevision) {
if (acknowledgedToken.current === treeReadyToken) {
return;
}
acknowledgedRevision.current = treeReadyRevision;
acknowledgedToken.current = treeReadyToken;
scenarioMonitor.completePhase(MetricScenario.DatabaseLoad, ApplicationMetricPhase.DatabaseTreeRendered);
}, [databaseTreeNodes, fetchSucceeded, treeReadyRevision]);
}, [databaseTreeNodes, fetchSucceeded, treeReadyToken]);
// Track Interactive phase
useInteractive(MetricScenario.DatabaseLoad);