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

[RC3] GC: Port recent telemetry changes #20787

Merged

Conversation

markfields
Copy link
Member

Port the following commits to RC3 branch:

…are used. (microsoft#20354)

Three changes here:

1. Include the full requested path as the id (not trimming to the
DataStore)
2. If a node is requested in the same session it was deleted from, log
the pkg path
3. Include a specific header value on the 404 response for deleted nodes

This will help confirm or refute some assumptions we're making in
current investigations, and potentially fill in some gaps.
We have received reports that some documents with tens of thousands of
unreferenced nodes take over a second for GC to initialize upon
connecting. We need to understand how often this is happening and what
the distribution of unreferenced node counts v. duration is like.
…microsoft#20725)

Key logic changes:
* GC's `nodeUpdated` function only accepts Blob or DataStore paths (no
subDataStore).
* This is because subDataStore paths may not be tracked/understood by
GC. So we use the DataStore path to check Tombstone/Inactive status.
* Note that the original subDataStore path is available for logging
since the original `request` is passed in
* `GCTelemetryTracker.nodeUsed` takes in a `trackedId` param, separate
from the props.
  * `trackedId` would be the DataStore ID in a SubDataStore case
  * It's used to look up the UnreferencedNodeStateTracker

There is some cleanup type stuff in the diff as well, in the areas I was
working. Very light refactoring and comments mostly.

I added tests to request inactive or Tombstoned SubDataStore paths, and
checked the logs to see that we get the logs we are currently missing in
these cases.
@markfields markfields enabled auto-merge (squash) April 22, 2024 22:40
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +4.29 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 452.29 KB 453.35 KB +1.07 KB
azureClient.js 545.36 KB 546.43 KB +1.07 KB
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 255.17 KB 256.25 KB +1.08 KB
fluidFramework.js 339.39 KB 339.39 KB No change
loader.js 129.89 KB 129.89 KB No change
map.js 41.35 KB 41.35 KB No change
matrix.js 143.62 KB 143.62 KB No change
odspClient.js 513.82 KB 514.89 KB +1.07 KB
odspDriver.js 97.52 KB 97.52 KB No change
odspPrefetchSnapshot.js 42.39 KB 42.39 KB No change
sharedString.js 161.39 KB 161.39 KB No change
sharedTree.js 339.39 KB 339.39 KB No change
Total Size 3.14 MB 3.14 MB +4.29 KB

Baseline commit: 75a9de2

Generated by 🚫 dangerJS against ac3f650

@markfields markfields merged commit 811dcc8 into microsoft:release/client/2.0.0-rc.3.0 Apr 22, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants