From 04704767b8d9e5d6c2b0a2bfbecf71e8d5680e4f Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:47:17 +0200 Subject: [PATCH 1/7] MILAB-6303: add tree-sync duplication metrics Extend the MI_LOG_TREE_STAT counters with per-cycle re-fetch classification computed in updateFromResourceData: new, changed and unchanged resources, wasted downlink bytes, BFS fetches spent on unchanged resources, and changes that re-streamed stable metadata. Quantifies how much of each poll re-fetches resources the client already holds. Reuses the existing changed flag, no extra work per resource. --- .changeset/tree-sync-duplication-metrics.md | 5 ++ lib/node/pl-tree/src/state.ts | 51 ++++++++++++++++++++- lib/node/pl-tree/src/sync.ts | 22 +++++++-- lib/node/pl-tree/src/synchronized_tree.ts | 2 +- 4 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 .changeset/tree-sync-duplication-metrics.md diff --git a/.changeset/tree-sync-duplication-metrics.md b/.changeset/tree-sync-duplication-metrics.md new file mode 100644 index 0000000000..3712fad976 --- /dev/null +++ b/.changeset/tree-sync-duplication-metrics.md @@ -0,0 +1,5 @@ +--- +"@milaboratories/pl-tree": patch +--- + +Add cross-cycle duplication counters to tree-sync stats (MI_LOG_TREE_STAT): new/changed/unchanged resources, wasted downlink bytes, BFS fetches spent on unchanged resources, and changes that re-streamed stable metadata. Quantifies how much of each poll re-fetches resources the client already holds. diff --git a/lib/node/pl-tree/src/state.ts b/lib/node/pl-tree/src/state.ts index 24dd4018a4..43bdba0d9e 100644 --- a/lib/node/pl-tree/src/state.ts +++ b/lib/node/pl-tree/src/state.ts @@ -68,6 +68,30 @@ export type ResourceDataWithFinalState = ResourceData & { finalState: boolean; }; +/** Cross-cycle re-fetch duplication counters, populated by {@link PlTreeState.updateFromResourceData}. + * Separates genuinely new/changed resources from redundant re-fetches of unchanged ones + * (the delta-skip opportunity: intra-cycle dedup already happens in the loader, so all waste here + * is cross-cycle). */ +export interface ResourceUpdateStat { + /** Resources seen for the first time in this update. */ + resourcesNew: number; + /** Resources already held whose state actually changed. */ + resourcesChanged: number; + /** Resources already held that came back unchanged: a pure duplicate re-fetch. */ + resourcesUnchanged: number; + /** data + KV bytes of the unchanged bucket: wasted downlink. */ + bytesUnchanged: number; + /** Changed resources whose immutable metadata (type, kind, field set) did not change, + * i.e. metadata re-streamed even though only a value or flag flipped. */ + metadataStableChanged: number; + /** Per-resource fetches spent on unchanged resources (BFS only; streaming re-sends them + * inside one stream, so this stays 0). */ + bfsRequestsWasted: number; + /** Whether the current load used backend streaming; set by the loader, read here to + * attribute {@link bfsRequestsWasted}. */ + usedStreaming: boolean; +} + /** Never store instances of this class, always get fresh instance from {@link PlTreeState} */ export class PlTreeResource implements ResourceDataWithFinalState { /** Tracks number of other resources referencing this resource. Used to perform garbage collection in tree patching procedure */ @@ -451,7 +475,11 @@ export class PlTreeState { return res; } - updateFromResourceData(resourceData: ExtendedResourceData[], allowOrphanInputs: boolean = false) { + updateFromResourceData( + resourceData: ExtendedResourceData[], + allowOrphanInputs: boolean = false, + stat?: ResourceUpdateStat, + ) { this.checkValid(); // All resources for which recount should be incremented, first are aggregated in this list @@ -461,6 +489,11 @@ export class PlTreeState { // patching / creating resources for (const rd of resourceData) { let resource = this.resources.get(rd.id); + const held = resource !== undefined; + let changed = false; + // Structural/metadata change (new or removed field). type and kind are readonly, so + // they never change; this flag isolates value/flag-only changes from real metadata churn. + let metadataChanged = false; const statBeforeMutation = resource?.basicState; const unexpectedTransitionError = (reason: string): never => { @@ -480,7 +513,6 @@ export class PlTreeState { if (resource.finalState) unexpectedTransitionError("resource state can\t be updated after it is marked as final"); - let changed = false; // updating resource version, even if it was not changed resource.version += 1; @@ -552,6 +584,7 @@ export class PlTreeState { resource.fieldsMap.set(fd.name, field); changed = true; + metadataChanged = true; } else { // change of old field @@ -640,6 +673,7 @@ export class PlTreeState { `dynamic field ${fieldName} removed from ${resourceIdToString(resource!.id)}`, ); fields.delete(fieldName); + metadataChanged = true; if (isNotNullSignedResourceId(field.value)) decrementRefs.push(field.value); if (isNotNullSignedResourceId(field.error)) decrementRefs.push(field.error); @@ -756,6 +790,19 @@ export class PlTreeState { this.resources.set(resource.id, resource); this.resourcesAdded.markChanged(`new resource ${resourceIdToString(resource.id)} added`); } + + if (stat) { + if (!held) stat.resourcesNew++; + else if (changed) { + stat.resourcesChanged++; + if (!metadataChanged) stat.metadataStableChanged++; + } else { + stat.resourcesUnchanged++; + stat.bytesUnchanged += rd.data?.length ?? 0; + for (const kv of rd.kv) stat.bytesUnchanged += kv.value.length; + if (!stat.usedStreaming) stat.bfsRequestsWasted++; + } + } } // applying refCount increments diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index 3e640bfcc6..2695f8a8bd 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -8,7 +8,7 @@ import type { } from "@milaboratories/pl-client"; import Denque from "denque"; import { hasCapability, isNullSignedResourceId } from "@milaboratories/pl-client"; -import type { ExtendedResourceData, PlTreeState } from "./state"; +import type { ExtendedResourceData, PlTreeState, ResourceUpdateStat } from "./state"; import { ConcurrencyLimitingExecutor, msToHumanReadable } from "@milaboratories/ts-helpers"; /** Applied to list of fields in resource data. */ @@ -70,7 +70,7 @@ export function constructTreeLoadingRequest( }; } -export type TreeLoadingStat = { +export type TreeLoadingStat = ResourceUpdateStat & { requests: number; roundTrips: number; retrievedResources: number; @@ -101,6 +101,13 @@ export function initialTreeLoadingStat(): TreeLoadingStat { millisSpent: 0, stopMarkersSkipped: 0, stopMarkerFollowUpRoundTrips: 0, + resourcesNew: 0, + resourcesChanged: 0, + resourcesUnchanged: 0, + bytesUnchanged: 0, + metadataStableChanged: 0, + bfsRequestsWasted: 0, + usedStreaming: false, }; } @@ -116,7 +123,14 @@ export function formatTreeLoadingStat(stat: TreeLoadingStat): string { result += `Pruned fields: ${stat.prunedFields}\n`; result += `Final resources skipped: ${stat.finalResourcesSkipped}\n`; result += `Stop markers skipped: ${stat.stopMarkersSkipped}\n`; - result += `Stop marker follow-up round-trips: ${stat.stopMarkerFollowUpRoundTrips}`; + result += `Stop marker follow-up round-trips: ${stat.stopMarkerFollowUpRoundTrips}\n`; + result += `New resources: ${stat.resourcesNew}\n`; + result += `Changed resources: ${stat.resourcesChanged}\n`; + result += `Unchanged (duplicate re-fetch) resources: ${stat.resourcesUnchanged}\n`; + result += `Unchanged bytes (wasted downlink): ${stat.bytesUnchanged}\n`; + result += `Changed with stable metadata: ${stat.metadataStableChanged}\n`; + result += `BFS fetches wasted on unchanged: ${stat.bfsRequestsWasted}\n`; + result += `Used streaming: ${stat.usedStreaming}`; return result; } @@ -364,6 +378,8 @@ export async function loadTreeState( mode === "backend-streaming" || (mode === "auto" && supportsResourceTreeTraversal(capabilities)); + if (stats) stats.usedStreaming = wantsStreaming && supportsResourceTreeTraversal(capabilities); + if (wantsStreaming && !supportsResourceTreeTraversal(capabilities)) { const msg = "traversalMode=backend-streaming but backend lacks treeFilter:v2 capability; falling back to BFS"; diff --git a/lib/node/pl-tree/src/synchronized_tree.ts b/lib/node/pl-tree/src/synchronized_tree.ts index 18d183e253..26b0fabf04 100644 --- a/lib/node/pl-tree/src/synchronized_tree.ts +++ b/lib/node/pl-tree/src/synchronized_tree.ts @@ -276,7 +276,7 @@ export class SynchronizedTreeState { }, txOps, ); - this.state.updateFromResourceData(data, true); + this.state.updateFromResourceData(data, true, stats); } /** Discovery sync for shared-type seeds: re-polls `ListUserResources` (gRPC-only) and From 8e88ee8dba72c0ad06ee4292f0a9f46997f4328b Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:43:05 +0200 Subject: [PATCH 2/7] MILAB-6303: full tree-sync introspection + stop-rule follow-up fix Expand MI_LOG_TREE_STAT into full per-path introspection: streaming detail (rounds, resource/stop-marker frames, follow-up seeds, traverse-stopped), BFS detail (resources requested, not found), and a per-cycle change breakdown (fields added/removed/changed, kv, ready flips, locks, errors, duplicates resolved, resources marked final), alongside the existing cross-cycle duplication counters. Fix the resourceTree stop-marker follow-up to propagate traverseStopRules so the retry stops at the same boundaries instead of traversing the stop-marked subtrees unbounded. Assert that one follow-up round resolves every stop marker (throws if a deeper stop-marker chain remains, which would mean the retry must become a loop). --- .changeset/tree-sync-duplication-metrics.md | 2 +- lib/node/pl-tree/src/state.ts | 35 ++++++++++- lib/node/pl-tree/src/sync.ts | 67 ++++++++++++++++++--- 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/.changeset/tree-sync-duplication-metrics.md b/.changeset/tree-sync-duplication-metrics.md index 3712fad976..72ee895aa3 100644 --- a/.changeset/tree-sync-duplication-metrics.md +++ b/.changeset/tree-sync-duplication-metrics.md @@ -2,4 +2,4 @@ "@milaboratories/pl-tree": patch --- -Add cross-cycle duplication counters to tree-sync stats (MI_LOG_TREE_STAT): new/changed/unchanged resources, wasted downlink bytes, BFS fetches spent on unchanged resources, and changes that re-streamed stable metadata. Quantifies how much of each poll re-fetches resources the client already holds. +Instrument tree-sync stats (MI_LOG_TREE_STAT) for full introspection: cross-cycle duplication (new/changed/unchanged resources, wasted downlink bytes, stable-metadata re-streams), per-path detail (streaming rounds/frames/stop-markers, BFS requests and not-found), and a breakdown of what changed each cycle (fields, kv, ready, locks, finalization). Also fixes the resourceTree stop-marker follow-up to propagate traverseStopRules so the retry stops at the same boundaries instead of traversing unbounded, and asserts that one follow-up round resolves every stop marker. diff --git a/lib/node/pl-tree/src/state.ts b/lib/node/pl-tree/src/state.ts index 43bdba0d9e..b0e72c510e 100644 --- a/lib/node/pl-tree/src/state.ts +++ b/lib/node/pl-tree/src/state.ts @@ -90,6 +90,25 @@ export interface ResourceUpdateStat { /** Whether the current load used backend streaming; set by the loader, read here to * attribute {@link bfsRequestsWasted}. */ usedStreaming: boolean; + // Breakdown of what changed among resourcesChanged (a resource can bump several). + /** New fields appeared on a held resource. */ + fieldsAdded: number; + /** Dynamic fields disappeared from a held resource. */ + fieldsRemoved: number; + /** Existing field value pointer changed (a new value/result was attached). */ + fieldsChanged: number; + /** KV entries added, changed, or deleted. */ + kvChanged: number; + /** resourceReady flipped. */ + readyFlips: number; + /** inputsLocked or outputsLocked transitioned. */ + locksChanged: number; + /** An error resource was attached. */ + errorsAttached: number; + /** originalResourceId resolved (duplicate -> original). */ + duplicatesResolved: number; + /** Held resources that transitioned to final this update. */ + resourcesMarkedFinal: number; } /** Never store instances of this class, always get fresh instance from {@link PlTreeState} */ @@ -526,6 +545,7 @@ export class PlTreeState { `originalResourceId changed for ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.duplicatesResolved++; } // error @@ -538,6 +558,7 @@ export class PlTreeState { `error changed for ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.errorsAttached++; } // updating fields @@ -585,6 +606,7 @@ export class PlTreeState { changed = true; metadataChanged = true; + if (stat) stat.fieldsAdded++; } else { // change of old field @@ -629,6 +651,7 @@ export class PlTreeState { `field ${fd.name} value changed in ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.fieldsChanged++; } // field error @@ -674,6 +697,7 @@ export class PlTreeState { ); fields.delete(fieldName); metadataChanged = true; + if (stat) stat.fieldsRemoved++; if (isNotNullSignedResourceId(field.value)) decrementRefs.push(field.value); if (isNotNullSignedResourceId(field.error)) decrementRefs.push(field.error); @@ -692,6 +716,7 @@ export class PlTreeState { `inputs locked for ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.locksChanged++; } // outputsLocked @@ -703,6 +728,7 @@ export class PlTreeState { `outputs locked for ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.locksChanged++; } // ready flag @@ -718,6 +744,7 @@ export class PlTreeState { `ready flag changed to ${rd.resourceReady} for ${resourceIdToString(resource.id)}`, ); changed = true; + if (stat) stat.readyFlips++; } // syncing kv @@ -729,12 +756,14 @@ export class PlTreeState { kv.key, `kv added for ${resourceIdToString(resource.id)}: ${kv.key}`, ); + if (stat) stat.kvChanged++; } else if (Buffer.compare(current, kv.value) !== 0) { resource.kv.set(kv.key, kv.value); notEmpty(resource.kvChangedPerKey).markChanged( kv.key, `kv changed for ${resourceIdToString(resource.id)}: ${kv.key}`, ); + if (stat) stat.kvChanged++; } } @@ -750,6 +779,7 @@ export class PlTreeState { key, `kv deleted for ${resourceIdToString(resource!.id)}: ${key}`, ); + if (stat) stat.kvChanged++; } }); } @@ -757,7 +787,10 @@ export class PlTreeState { if (changed) { // if resource was changed, updating resource data version resource.dataVersion = resource.version; - if (this.isFinalPredicate(resource)) resource.markFinal(); + if (this.isFinalPredicate(resource)) { + resource.markFinal(); + if (stat) stat.resourcesMarkedFinal++; + } } } else { // creating new resource diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index 2695f8a8bd..c264c692f8 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -85,6 +85,20 @@ export type TreeLoadingStat = ResourceUpdateStat & { stopMarkersSkipped: number; /** Number of follow-up resourceTree() calls issued to resolve unknown stop markers. */ stopMarkerFollowUpRoundTrips: number; + /** Streaming (resourceTree) path: resourceTree() streams consumed (1, or 2 with a follow-up). */ + streamRounds: number; + /** Streaming path: resource frames received. */ + resourceFrames: number; + /** Streaming path: stopMarker frames received (both skipped and follow-up). */ + stopMarkerFrames: number; + /** Streaming path: stop markers that were not final locally and triggered a follow-up fetch. */ + stopMarkersFollowUp: number; + /** Streaming path: frames where the backend stopped traversal (final or traverseWasStopped). */ + traverseWasStoppedCount: number; + /** BFS path: resource states actually requested from the backend this cycle (after intra-cycle dedup). */ + bfsResourcesRequested: number; + /** BFS path: requested resources that no longer exist (undefined reply). */ + bfsResourcesNotFound: number; }; export function initialTreeLoadingStat(): TreeLoadingStat { @@ -101,6 +115,13 @@ export function initialTreeLoadingStat(): TreeLoadingStat { millisSpent: 0, stopMarkersSkipped: 0, stopMarkerFollowUpRoundTrips: 0, + streamRounds: 0, + resourceFrames: 0, + stopMarkerFrames: 0, + stopMarkersFollowUp: 0, + traverseWasStoppedCount: 0, + bfsResourcesRequested: 0, + bfsResourcesNotFound: 0, resourcesNew: 0, resourcesChanged: 0, resourcesUnchanged: 0, @@ -108,6 +129,15 @@ export function initialTreeLoadingStat(): TreeLoadingStat { metadataStableChanged: 0, bfsRequestsWasted: 0, usedStreaming: false, + fieldsAdded: 0, + fieldsRemoved: 0, + fieldsChanged: 0, + kvChanged: 0, + readyFlips: 0, + locksChanged: 0, + errorsAttached: 0, + duplicatesResolved: 0, + resourcesMarkedFinal: 0, }; } @@ -130,7 +160,9 @@ export function formatTreeLoadingStat(stat: TreeLoadingStat): string { result += `Unchanged bytes (wasted downlink): ${stat.bytesUnchanged}\n`; result += `Changed with stable metadata: ${stat.metadataStableChanged}\n`; result += `BFS fetches wasted on unchanged: ${stat.bfsRequestsWasted}\n`; - result += `Used streaming: ${stat.usedStreaming}`; + result += `Used streaming: ${stat.usedStreaming}\n`; + result += `[streaming] rounds: ${stat.streamRounds}, resource frames: ${stat.resourceFrames}, stop-marker frames: ${stat.stopMarkerFrames}, stop->follow-up: ${stat.stopMarkersFollowUp}, traverse-stopped: ${stat.traverseWasStoppedCount}\n`; + result += `[bfs] resources requested: ${stat.bfsResourcesRequested}, not found: ${stat.bfsResourcesNotFound}`; return result; } @@ -178,6 +210,7 @@ async function loadTreeStateViaBfs( } requested.add(rid); + if (stats) stats.bfsResourcesRequested++; pending.push( limiter.run(async () => { @@ -197,7 +230,10 @@ async function loadTreeStateViaBfs( roundTripToggle = true; } - if (resource === undefined) return undefined; + if (resource === undefined) { + if (stats) stats.bfsResourcesNotFound++; + return undefined; + } if (kv === undefined) throw new Error("Inconsistent replies"); return { ...resource, kv }; @@ -261,15 +297,21 @@ async function processResourceTreeStream( // should make a decision: has it already loaded the resource or should it be requested for get the latest state? for await (const frame of treeItems) { if (frame.frameKind === "stopMarker") { + if (stats) stats.stopMarkerFrames++; if (finalResources.has(frame.id)) { if (stats) stats.stopMarkersSkipped++; continue; } + if (stats) stats.stopMarkersFollowUp++; followUpSeeds.push(frame.id); continue; } // Normal resource frame. + if (stats) { + stats.resourceFrames++; + if (frame.traverseWasStopped) stats.traverseWasStoppedCount++; + } if (finalResources.has(frame.id)) { if (stats) stats.finalResourcesSkipped++; continue; @@ -331,7 +373,10 @@ async function loadTreeStateViaResourceTree( pruningFunction, stats, ); - if (stats) stats.roundTrips++; + if (stats) { + stats.roundTrips++; + stats.streamRounds++; + } // Client must request full resource tree in case when stop-marker seeds are returned, // to ensure all resources are loaded and stop-marker frames are processed. @@ -339,19 +384,23 @@ async function loadTreeStateViaResourceTree( const followUpItems = tx.resourceTree(followUpSeeds, { includeKv: true, fieldFilter, + traverseStopRules, }); - const { result: followUpResult } = await processResourceTreeStream( - followUpItems, - finalResources, - pruningFunction, - stats, - ); + const { result: followUpResult, followUpSeeds: unresolvedSeeds } = + await processResourceTreeStream(followUpItems, finalResources, pruningFunction, stats); + // Invariant: one follow-up round resolves every stop marker. Stop markers in the + // follow-up stream mean a deeper stop-marker chain than the two-round design handles. + if (unresolvedSeeds.length > 0) + throw new Error( + `resourceTree follow-up left ${unresolvedSeeds.length} unresolved stop-marker seeds: ${JSON.stringify(unresolvedSeeds)}`, + ); result.push(...followUpResult); if (stats) { logger?.info?.( `loadTreeStateViaResourceTree: follow-up request for ${followUpSeeds.length} stop-marker seeds: ${JSON.stringify(followUpSeeds)}`, ); stats.roundTrips++; + stats.streamRounds++; stats.stopMarkerFollowUpRoundTrips++; } } From 7fc04c5737f0325054c65ddf31bc25d2255fe6d0 Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:16:40 +0200 Subject: [PATCH 3/7] MILAB-6303: mark resource changed on KV add/modify/delete KV mutations previously did not set the changed flag, so they never bumped dataVersion, skipped isFinalPredicate re-evaluation, and were miscounted as unchanged re-fetches in the tree-sync stats. Set changed on KV add, modify, and delete so versioning, finality, and the duplication metrics are correct. Addresses PR review. --- .changeset/tree-sync-duplication-metrics.md | 2 +- lib/node/pl-tree/src/state.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.changeset/tree-sync-duplication-metrics.md b/.changeset/tree-sync-duplication-metrics.md index 72ee895aa3..128796fd40 100644 --- a/.changeset/tree-sync-duplication-metrics.md +++ b/.changeset/tree-sync-duplication-metrics.md @@ -2,4 +2,4 @@ "@milaboratories/pl-tree": patch --- -Instrument tree-sync stats (MI_LOG_TREE_STAT) for full introspection: cross-cycle duplication (new/changed/unchanged resources, wasted downlink bytes, stable-metadata re-streams), per-path detail (streaming rounds/frames/stop-markers, BFS requests and not-found), and a breakdown of what changed each cycle (fields, kv, ready, locks, finalization). Also fixes the resourceTree stop-marker follow-up to propagate traverseStopRules so the retry stops at the same boundaries instead of traversing unbounded, and asserts that one follow-up round resolves every stop marker. +Instrument tree-sync stats (MI_LOG_TREE_STAT) for full introspection: cross-cycle duplication (new/changed/unchanged resources, wasted downlink bytes, stable-metadata re-streams), per-path detail (streaming rounds/frames/stop-markers, BFS requests and not-found), and a breakdown of what changed each cycle (fields, kv, ready, locks, finalization). Also fixes the resourceTree stop-marker follow-up to propagate traverseStopRules so the retry stops at the same boundaries instead of traversing unbounded, and asserts that one follow-up round resolves every stop marker. KV additions, modifications, and deletions now mark the resource changed, so they bump dataVersion, re-evaluate finality, and are counted correctly. diff --git a/lib/node/pl-tree/src/state.ts b/lib/node/pl-tree/src/state.ts index b0e72c510e..e216f13e5e 100644 --- a/lib/node/pl-tree/src/state.ts +++ b/lib/node/pl-tree/src/state.ts @@ -756,6 +756,7 @@ export class PlTreeState { kv.key, `kv added for ${resourceIdToString(resource.id)}: ${kv.key}`, ); + changed = true; if (stat) stat.kvChanged++; } else if (Buffer.compare(current, kv.value) !== 0) { resource.kv.set(kv.key, kv.value); @@ -763,6 +764,7 @@ export class PlTreeState { kv.key, `kv changed for ${resourceIdToString(resource.id)}: ${kv.key}`, ); + changed = true; if (stat) stat.kvChanged++; } } @@ -779,6 +781,7 @@ export class PlTreeState { key, `kv deleted for ${resourceIdToString(resource!.id)}: ${key}`, ); + changed = true; if (stat) stat.kvChanged++; } }); From 23a3ee3b9c09b3f609bad177e6987436a4f57fb6 Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:48:45 +0200 Subject: [PATCH 4/7] MILAB-6303: disable resourceTree follow-up assertion Comment out the throw on unresolved stop-marker seeds after the follow-up round while the deeper-chain case is under investigation. Keep the traverseStopRules propagation and leave re-enable instructions inline. --- lib/node/pl-tree/src/sync.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index c264c692f8..419e92a695 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -386,14 +386,18 @@ async function loadTreeStateViaResourceTree( fieldFilter, traverseStopRules, }); - const { result: followUpResult, followUpSeeds: unresolvedSeeds } = - await processResourceTreeStream(followUpItems, finalResources, pruningFunction, stats); - // Invariant: one follow-up round resolves every stop marker. Stop markers in the - // follow-up stream mean a deeper stop-marker chain than the two-round design handles. - if (unresolvedSeeds.length > 0) - throw new Error( - `resourceTree follow-up left ${unresolvedSeeds.length} unresolved stop-marker seeds: ${JSON.stringify(unresolvedSeeds)}`, - ); + const { result: followUpResult } = await processResourceTreeStream( + followUpItems, + finalResources, + pruningFunction, + stats, + ); + // Invariant (disabled, under investigation): one follow-up round should resolve every + // stop marker; a stop marker in the follow-up stream means a deeper chain than the + // two-round design handles. Re-enable by capturing followUpSeeds above and throwing: + // const { followUpSeeds: unresolvedSeeds } = await processResourceTreeStream(...); + // if (unresolvedSeeds.length > 0) + // throw new Error(`resourceTree follow-up left ${unresolvedSeeds.length} unresolved stop-marker seeds: ${JSON.stringify(unresolvedSeeds)}`); result.push(...followUpResult); if (stats) { logger?.info?.( From 0f88d86f6b3aa6e94e11821e3f52028bb67eb7e2 Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:54:44 +0200 Subject: [PATCH 5/7] MILAB-6303: loop resourceTree follow-up to resolve deep stop-marker chains Propagating traverseStopRules to the follow-up bounded the retry but left stop-marker chains deeper than one level unresolved: a loaded resource referenced a deeper stop-marked resource that was never fetched, so updateFromResourceData threw "orphan resource" on open. Loop the follow-up (each round with the same stop rules, deduping fetched ids) until no stop-marker seeds remain, so everything referenced is loaded. Replaces the disabled one-shot assertion. --- lib/node/pl-tree/src/sync.ts | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index 419e92a695..6e577e7052 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -378,35 +378,39 @@ async function loadTreeStateViaResourceTree( stats.streamRounds++; } - // Client must request full resource tree in case when stop-marker seeds are returned, - // to ensure all resources are loaded and stop-marker frames are processed. - if (followUpSeeds.length > 0) { - const followUpItems = tx.resourceTree(followUpSeeds, { + // Resolve stop-marker seeds. Each follow-up round applies the same traverseStopRules, so a + // deep tree can surface a further layer of stop markers; loop until none remain. Otherwise + // resources referenced by loaded ones stay unloaded and updateFromResourceData throws + // "orphan resource". Dedup fetched ids so shared refs / diamonds cannot loop forever + // (the id set is finite, so the loop terminates). + let pendingSeeds = followUpSeeds; + const fetchedSeeds = new Set(); + while (pendingSeeds.length > 0) { + const roundSeeds = pendingSeeds.filter((id) => !fetchedSeeds.has(id)); + if (roundSeeds.length === 0) break; + for (const id of roundSeeds) fetchedSeeds.add(id); + + const followUpItems = tx.resourceTree(roundSeeds, { includeKv: true, fieldFilter, traverseStopRules, }); - const { result: followUpResult } = await processResourceTreeStream( + const { result: followUpResult, followUpSeeds: nextSeeds } = await processResourceTreeStream( followUpItems, finalResources, pruningFunction, stats, ); - // Invariant (disabled, under investigation): one follow-up round should resolve every - // stop marker; a stop marker in the follow-up stream means a deeper chain than the - // two-round design handles. Re-enable by capturing followUpSeeds above and throwing: - // const { followUpSeeds: unresolvedSeeds } = await processResourceTreeStream(...); - // if (unresolvedSeeds.length > 0) - // throw new Error(`resourceTree follow-up left ${unresolvedSeeds.length} unresolved stop-marker seeds: ${JSON.stringify(unresolvedSeeds)}`); result.push(...followUpResult); if (stats) { logger?.info?.( - `loadTreeStateViaResourceTree: follow-up request for ${followUpSeeds.length} stop-marker seeds: ${JSON.stringify(followUpSeeds)}`, + `loadTreeStateViaResourceTree: follow-up request for ${roundSeeds.length} stop-marker seeds: ${JSON.stringify(roundSeeds)}`, ); stats.roundTrips++; stats.streamRounds++; stats.stopMarkerFollowUpRoundTrips++; } + pendingSeeds = nextSeeds; } return result; From f329cc38734604d6804548c8e0dac3713e1b3c4d Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:01:24 +0200 Subject: [PATCH 6/7] MILAB-6303: fetch stop-marker follow-up without stop rules (fix orphan) Reapplying traverseStopRules on the follow-up re-flagged the very resources being retried as stop markers (the rule is finality-based and they are server-final), so their state never loaded and resources referencing them threw "orphan resource" on open. Fetch follow-up seeds plainly; keep the loop only to resolve any residual stop markers. Update the changeset accordingly. --- .changeset/tree-sync-duplication-metrics.md | 2 +- lib/node/pl-tree/src/sync.ts | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.changeset/tree-sync-duplication-metrics.md b/.changeset/tree-sync-duplication-metrics.md index 128796fd40..7d7a6f91ec 100644 --- a/.changeset/tree-sync-duplication-metrics.md +++ b/.changeset/tree-sync-duplication-metrics.md @@ -2,4 +2,4 @@ "@milaboratories/pl-tree": patch --- -Instrument tree-sync stats (MI_LOG_TREE_STAT) for full introspection: cross-cycle duplication (new/changed/unchanged resources, wasted downlink bytes, stable-metadata re-streams), per-path detail (streaming rounds/frames/stop-markers, BFS requests and not-found), and a breakdown of what changed each cycle (fields, kv, ready, locks, finalization). Also fixes the resourceTree stop-marker follow-up to propagate traverseStopRules so the retry stops at the same boundaries instead of traversing unbounded, and asserts that one follow-up round resolves every stop marker. KV additions, modifications, and deletions now mark the resource changed, so they bump dataVersion, re-evaluate finality, and are counted correctly. +Instrument tree-sync stats (MI_LOG_TREE_STAT) for full introspection: cross-cycle duplication (new/changed/unchanged resources, wasted downlink bytes, stable-metadata re-streams), per-path detail (streaming rounds/frames/stop-markers, BFS requests and not-found), and a breakdown of what changed each cycle (fields, kv, ready, locks, finalization). The resourceTree stop-marker follow-up now loops until every stop-marker seed is resolved (deduping fetched ids), so deep trees fully load instead of leaving referenced resources as orphans. KV additions, modifications, and deletions now mark the resource changed, so they bump dataVersion, re-evaluate finality, and are counted correctly. diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index 6e577e7052..cce6f7dc25 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -378,11 +378,10 @@ async function loadTreeStateViaResourceTree( stats.streamRounds++; } - // Resolve stop-marker seeds. Each follow-up round applies the same traverseStopRules, so a - // deep tree can surface a further layer of stop markers; loop until none remain. Otherwise - // resources referenced by loaded ones stay unloaded and updateFromResourceData throws - // "orphan resource". Dedup fetched ids so shared refs / diamonds cannot loop forever - // (the id set is finite, so the loop terminates). + // Resolve stop-marker seeds by fetching them (see the note below on why the stop rule is not + // reapplied). Loop in case a fetch still yields stop markers; dedup fetched ids so shared refs + // or diamonds cannot loop forever (the id set is finite, so the loop terminates). A referenced + // resource left unloaded would make updateFromResourceData throw "orphan resource". let pendingSeeds = followUpSeeds; const fetchedSeeds = new Set(); while (pendingSeeds.length > 0) { @@ -390,10 +389,12 @@ async function loadTreeStateViaResourceTree( if (roundSeeds.length === 0) break; for (const id of roundSeeds) fetchedSeeds.add(id); + // No traverseStopRules here on purpose: the seeds are the stop-marked resources, and the + // stop rule (finality-based) would re-flag them as stop markers instead of loading their + // state, leaving resources that reference them as orphans. The retry must fetch them plainly. const followUpItems = tx.resourceTree(roundSeeds, { includeKv: true, fieldFilter, - traverseStopRules, }); const { result: followUpResult, followUpSeeds: nextSeeds } = await processResourceTreeStream( followUpItems, From 8b226cf93d3eae7ae1037a5509c0439c4c99a32d Mon Sep 17 00:00:00 2001 From: xnacly <47723417+xnacly@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:21:09 +0200 Subject: [PATCH 7/7] MILAB-6303: address review - options object + template literal Per review: replace the positional boolean arg on updateFromResourceData with an options object ({ allowOrphanInputs, stat }), updating the one caller; and build formatTreeLoadingStat as a single template literal instead of concatenating. --- lib/node/pl-tree/src/state.ts | 4 +-- lib/node/pl-tree/src/sync.ts | 43 +++++++++++------------ lib/node/pl-tree/src/synchronized_tree.ts | 2 +- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/lib/node/pl-tree/src/state.ts b/lib/node/pl-tree/src/state.ts index e216f13e5e..b4856c3127 100644 --- a/lib/node/pl-tree/src/state.ts +++ b/lib/node/pl-tree/src/state.ts @@ -496,9 +496,9 @@ export class PlTreeState { updateFromResourceData( resourceData: ExtendedResourceData[], - allowOrphanInputs: boolean = false, - stat?: ResourceUpdateStat, + opts: { allowOrphanInputs?: boolean; stat?: ResourceUpdateStat } = {}, ) { + const { allowOrphanInputs = false, stat } = opts; this.checkValid(); // All resources for which recount should be incremented, first are aggregated in this list diff --git a/lib/node/pl-tree/src/sync.ts b/lib/node/pl-tree/src/sync.ts index cce6f7dc25..284f5306ce 100644 --- a/lib/node/pl-tree/src/sync.ts +++ b/lib/node/pl-tree/src/sync.ts @@ -142,28 +142,27 @@ export function initialTreeLoadingStat(): TreeLoadingStat { } export function formatTreeLoadingStat(stat: TreeLoadingStat): string { - let result = `Requests: ${stat.requests}\n`; - result += `Total time: ${msToHumanReadable(stat.millisSpent)}\n`; - result += `Round-trips: ${stat.roundTrips}\n`; - result += `Resources: ${stat.retrievedResources}\n`; - result += `Fields: ${stat.retrievedFields}\n`; - result += `KV: ${stat.retrievedKeyValues}\n`; - result += `Data Bytes: ${stat.retrievedResourceDataBytes}\n`; - result += `KV Bytes: ${stat.retrievedKeyValueBytes}\n`; - result += `Pruned fields: ${stat.prunedFields}\n`; - result += `Final resources skipped: ${stat.finalResourcesSkipped}\n`; - result += `Stop markers skipped: ${stat.stopMarkersSkipped}\n`; - result += `Stop marker follow-up round-trips: ${stat.stopMarkerFollowUpRoundTrips}\n`; - result += `New resources: ${stat.resourcesNew}\n`; - result += `Changed resources: ${stat.resourcesChanged}\n`; - result += `Unchanged (duplicate re-fetch) resources: ${stat.resourcesUnchanged}\n`; - result += `Unchanged bytes (wasted downlink): ${stat.bytesUnchanged}\n`; - result += `Changed with stable metadata: ${stat.metadataStableChanged}\n`; - result += `BFS fetches wasted on unchanged: ${stat.bfsRequestsWasted}\n`; - result += `Used streaming: ${stat.usedStreaming}\n`; - result += `[streaming] rounds: ${stat.streamRounds}, resource frames: ${stat.resourceFrames}, stop-marker frames: ${stat.stopMarkerFrames}, stop->follow-up: ${stat.stopMarkersFollowUp}, traverse-stopped: ${stat.traverseWasStoppedCount}\n`; - result += `[bfs] resources requested: ${stat.bfsResourcesRequested}, not found: ${stat.bfsResourcesNotFound}`; - return result; + return `Requests: ${stat.requests} +Total time: ${msToHumanReadable(stat.millisSpent)} +Round-trips: ${stat.roundTrips} +Resources: ${stat.retrievedResources} +Fields: ${stat.retrievedFields} +KV: ${stat.retrievedKeyValues} +Data Bytes: ${stat.retrievedResourceDataBytes} +KV Bytes: ${stat.retrievedKeyValueBytes} +Pruned fields: ${stat.prunedFields} +Final resources skipped: ${stat.finalResourcesSkipped} +Stop markers skipped: ${stat.stopMarkersSkipped} +Stop marker follow-up round-trips: ${stat.stopMarkerFollowUpRoundTrips} +New resources: ${stat.resourcesNew} +Changed resources: ${stat.resourcesChanged} +Unchanged (duplicate re-fetch) resources: ${stat.resourcesUnchanged} +Unchanged bytes (wasted downlink): ${stat.bytesUnchanged} +Changed with stable metadata: ${stat.metadataStableChanged} +BFS fetches wasted on unchanged: ${stat.bfsRequestsWasted} +Used streaming: ${stat.usedStreaming} +[streaming] rounds: ${stat.streamRounds}, resource frames: ${stat.resourceFrames}, stop-marker frames: ${stat.stopMarkerFrames}, stop->follow-up: ${stat.stopMarkersFollowUp}, traverse-stopped: ${stat.traverseWasStoppedCount} +[bfs] resources requested: ${stat.bfsResourcesRequested}, not found: ${stat.bfsResourcesNotFound}`; } function supportsResourceTreeTraversal(capabilities: readonly string[] = []): boolean { diff --git a/lib/node/pl-tree/src/synchronized_tree.ts b/lib/node/pl-tree/src/synchronized_tree.ts index 26b0fabf04..501fab74aa 100644 --- a/lib/node/pl-tree/src/synchronized_tree.ts +++ b/lib/node/pl-tree/src/synchronized_tree.ts @@ -276,7 +276,7 @@ export class SynchronizedTreeState { }, txOps, ); - this.state.updateFromResourceData(data, true, stats); + this.state.updateFromResourceData(data, { allowOrphanInputs: true, stat: stats }); } /** Discovery sync for shared-type seeds: re-polls `ListUserResources` (gRPC-only) and