Revert "feat(container-runtime): enable batchId tracking by default with kill-switch and perf telemetry (#27216)"#27256
Merged
WillieHabi merged 1 commit intoMay 7, 2026
Conversation
…ith kill-switch and perf telemetry (microsoft#27216)" This reverts commit 085f51b.
Contributor
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (433 lines, 11 files), I've queued these reviewers:
How this works
|
Contributor
There was a problem hiding this comment.
Pull request overview
Reverts PR #27216 by restoring batchId tracking to be opt-in (default false) and removing the default-on rollout behavior and kill-switch wiring. This updates container runtime behavior back to the previous gating model and adjusts snapshot/test infrastructure accordingly.
Changes:
- Restore opt-in behavior for batchId tracking (
Fluid.ContainerRuntime.enableBatchIdTracking) and remove kill-switch-based default enablement. - Simplify snapshot validation and replay-tool snapshot comparison (packageVersion normalization + remove special-casing for
.recentBatchInfo). - Update runtime and end-to-end tests to reflect the reverted defaults and telemetry expectations.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tools/replay-tool/src/index.ts | Removes export of normalizePackageVersions after reverting snapshot-comparison behavior. |
| packages/tools/replay-tool/src/helpers.ts | Inlines packageVersion normalization into snapshot comparison logic. |
| packages/test/test-end-to-end-tests/src/test/fewerBatches.spec.ts | Removes test-only kill-switch usage now that default behavior is reverted. |
| packages/test/test-end-to-end-tests/src/test/batching.spec.ts | Updates config used during batching tests to align with reverted gating. |
| packages/test/snapshots/src/validateSnapshots.ts | Removes .recentBatchInfo-ignoring comparison path and simplifies snapshot callback. |
| packages/test/functional-tests/src/test/containerRuntime.spec.ts | Updates constructed test ops to reflect reverted runtime expectations. |
| packages/runtime/container-runtime/src/test/opLifecycle/duplicateBatchDetector.spec.ts | Updates expectations for telemetry emitted by DuplicateBatchDetector after revert. |
| packages/runtime/container-runtime/src/test/containerRuntime.spec.ts | Updates batch-id stamping/detection tests to match opt-in behavior. |
| packages/runtime/container-runtime/src/opLifecycle/duplicateBatchDetector.ts | Removes per-window perf counters; retains only recent-batch count telemetry. |
| packages/runtime/container-runtime/src/containerRuntime.ts | Restores opt-in batchId tracking enablement logic and allocates DuplicateBatchDetector conditionally. |
Comment on lines
+1894
to
+1896
| this.mc.config.getBoolean("Fluid.Container.enableOfflineFull") ?? | ||
| this.mc.config.getBoolean("Fluid.ContainerRuntime.enableBatchIdTracking") ?? | ||
| false; |
Comment on lines
+1898
to
1902
| if (this.batchIdTrackingEnabled && this._flushMode !== FlushMode.TurnBased) { | ||
| const error = new UsageError("Offline mode is only supported in turn-based mode"); | ||
| this.closeFn(error); | ||
| throw error; | ||
| } |
|
|
||
| // DuplicateBatchDetector maintains a cache of all batchIds/sequenceNumbers within the | ||
| // collab window. Skip allocating it when batchId tracking is off. | ||
| // DuplicateBatchDetection is only enabled if Offline Load is enabled |
Comment on lines
103
to
111
| let testContainerConfig: ITestContainerConfig = { | ||
| fluidDataObjectType: DataObjectFactoryType.Test, | ||
| registry, | ||
| loaderProps: { | ||
| configProvider: configProvider({ | ||
| "Fluid.Container.enableOfflineFull": true, | ||
| "Fluid.ContainerRuntime.enableBatchIdTracking": true, | ||
| }), | ||
| }, | ||
| }; |
Comment on lines
+484
to
+490
| for (const enableBatchIdTracking of [true, undefined]) | ||
| it("Replaying ops should resend in correct order, with batch ID if applicable", async () => { | ||
| const { runtime } = await ContainerRuntime.loadRuntime2({ | ||
| context: getMockContext({ | ||
| settings: variant.settings, | ||
| settings: { | ||
| "Fluid.ContainerRuntime.enableBatchIdTracking": enableBatchIdTracking, // batchId only stamped if true | ||
| }, |
| containerErrors.push(error); | ||
| } | ||
| }, | ||
| it("Can roundrip DuplicateBatchDetector state through summary/snapshot", async () => { |
Comment on lines
+2396
to
+2403
| // Duplicate Batch Detection requires OfflineLoad enabled | ||
| const settings_enableOfflineLoad = { | ||
| "Fluid.ContainerRuntime.enableBatchIdTracking": true, | ||
| }; | ||
| await assert.rejects( | ||
| ContainerRuntime.loadRuntime2({ | ||
| context: context as IContainerContext, | ||
| registry: new FluidDataStoreRegistry([]), | ||
| existing: false, | ||
| runtimeOptions: { flushMode: FlushMode.Immediate }, | ||
| provideEntryPoint: mockProvideEntryPoint, | ||
| }), | ||
| (error: Error) => error instanceof UsageError, | ||
| ); | ||
| assert.strictEqual(containerErrors.length, 1); | ||
| }); | ||
|
|
||
| it("Can roundtrip DuplicateBatchDetector state through summary/snapshot", async () => { | ||
| // Duplicate Batch Detection is on by default in TurnBased mode. | ||
| const { runtime: containerRuntime } = await ContainerRuntime.loadRuntime2({ | ||
| context: getMockContext() as IContainerContext, | ||
| context: getMockContext({ | ||
| settings: settings_enableOfflineLoad, | ||
| }) as IContainerContext, |
Comment on lines
+89
to
+90
| * The packageVersion of the snapshot could be different from the reference snapshot. Replace all package | ||
| * package versions with X before we compare them. |
anthony-murphy
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Reverts #27216 ("feat(container-runtime): enable batchId tracking by default with kill-switch and perf telemetry").
Restores the prior behavior where
batchIdTrackingEnableddefaults tofalseand theFluid.ContainerRuntime.enableBatchIdTrackingopt-in flag continues to control the feature. TheFluid.ContainerRuntime.DisableBatchIdTrackingkill-switch added by #27216 is removed along with the default-on rollout.Reviewer Guidance
The review process is outlined on this wiki page.