Remove reentrantBatchGroupingEnabled (#23493)

## Description For the work in [#8124](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/8124) to be completed, we need to remove the ability for reentrant batches to configure whether grouping is on or off, it will now be on by default. Acceptance Criteria: Fluid users can no longer configure reentrantBatchGroupingEnabled Execution Plan: Remove reentrantBatchGroupingEnabled configuration from containerRuntime.ts and opGroupingManager.ts. ## Reviewer Guidance Let me know if there's a better way to accomplish the task. Fixes: [AB#27892](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/27892)
microsoft · Jan 8, 2025 · c15a136 · c15a136
1 parent fe8279c
commit c15a136
Show file tree

Hide file tree

Showing 5 changed files with 4 additions and 36 deletions.
diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts
@@ -1654,9 +1654,6 @@ export class ContainerRuntime
 				groupedBatchingEnabled: this.groupedBatchingEnabled,
 				opCountThreshold:
 					this.mc.config.getNumber("Fluid.ContainerRuntime.GroupedBatchingOpCount") ?? 2,
-				reentrantBatchGroupingEnabled:
-					this.mc.config.getBoolean("Fluid.ContainerRuntime.GroupedBatchingReentrancy") ??
-					true,
 			},
 			this.mc.logger,
 		);

diff --git a/packages/runtime/container-runtime/src/opLifecycle/opGroupingManager.ts b/packages/runtime/container-runtime/src/opLifecycle/opGroupingManager.ts
@@ -35,7 +35,6 @@ export function isGroupedBatch(op: ISequencedDocumentMessage): boolean {
 export interface OpGroupingManagerConfig {
 	readonly groupedBatchingEnabled: boolean;
 	readonly opCountThreshold: number;
-	readonly reentrantBatchGroupingEnabled: boolean;
 }
 
 export class OpGroupingManager {
@@ -156,9 +155,8 @@ export class OpGroupingManager {
 			this.config.groupedBatchingEnabled &&
 			// The number of ops in the batch must surpass the configured threshold
 			// or be empty (to allow for empty batches to be grouped)
-			(batch.messages.length === 0 || batch.messages.length >= this.config.opCountThreshold) &&
-			// Support for reentrant batches must be explicitly enabled
-			(this.config.reentrantBatchGroupingEnabled || batch.hasReentrantOps !== true)
+			(batch.messages.length === 0 || batch.messages.length >= this.config.opCountThreshold)
+			// Support for reentrant batches will be on by default
 		);
 	}
 }
diff --git a/packages/runtime/container-runtime/src/test/opLifecycle/OpGroupingManager.spec.ts b/packages/runtime/container-runtime/src/test/opLifecycle/OpGroupingManager.spec.ts
@@ -57,23 +57,19 @@ describe("OpGroupingManager", () => {
 		const options: ConfigOption[] = [
 			{ enabled: false, expectedResult: false },
 			{ enabled: true, tooSmall: true, expectedResult: false },
-			{ enabled: true, reentrant: true, expectedResult: false },
-			{ enabled: true, reentrant: true, reentryEnabled: true, expectedResult: true },
+			{ enabled: true, reentrant: true, expectedResult: true },
 			{ enabled: true, expectedResult: true },
 		];
 
 		options.forEach((option) => {
 			it(`shouldGroup: groupedBatchingEnabled [${option.enabled}] tooSmall [${
 				option.tooSmall === true
-			}] reentrant [${option.reentrant === true}] reentryEnabled [${
-				option.reentryEnabled === true
-			}]`, () => {
+			}] reentrant [${option.reentrant === true}]`, () => {
 				assert.strictEqual(
 					new OpGroupingManager(
 						{
 							groupedBatchingEnabled: option.enabled,
 							opCountThreshold: option.tooSmall === true ? 10 : 2,
-							reentrantBatchGroupingEnabled: option.reentryEnabled ?? false,
 						},
 						mockLogger,
 					).shouldGroup(createBatch(5, option.reentrant)),
@@ -90,7 +86,6 @@ describe("OpGroupingManager", () => {
 					{
 						groupedBatchingEnabled: false,
 						opCountThreshold: 2,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				).groupBatch(createBatch(5));
@@ -102,7 +97,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: false,
 				},
 				mockLogger,
 			).groupBatch(createBatch(5));
@@ -123,7 +117,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: false,
 				},
 				mockLogger,
 			).groupBatch(createBatch(5, false, false, batchId));
@@ -137,7 +130,6 @@ describe("OpGroupingManager", () => {
 					{
 						groupedBatchingEnabled: false,
 						opCountThreshold: 2,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				).createEmptyGroupedBatch("resubmittingBatchId", 0);
@@ -150,7 +142,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: false,
 				},
 				mockLogger,
 			).createEmptyGroupedBatch(batchId, 0);
@@ -169,7 +160,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: false,
 				},
 				mockLogger,
 			).shouldGroup({
@@ -187,7 +177,6 @@ describe("OpGroupingManager", () => {
 					{
 						groupedBatchingEnabled: true,
 						opCountThreshold: 10,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				).groupBatch(createBatch(5));
@@ -200,7 +189,6 @@ describe("OpGroupingManager", () => {
 					{
 						groupedBatchingEnabled: true,
 						opCountThreshold: 2,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				).groupBatch(createBatch(5, false, true));
@@ -213,7 +201,6 @@ describe("OpGroupingManager", () => {
 					{
 						groupedBatchingEnabled: true,
 						opCountThreshold: 2,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				).groupBatch(createBatch(5, false, true, "batchId"));
@@ -227,7 +214,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 				mockLogger,
 			);
@@ -280,7 +266,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 				mockLogger,
 			);
@@ -299,7 +284,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: false,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 				mockLogger,
 			);
@@ -344,7 +328,6 @@ describe("OpGroupingManager", () => {
 				{
 					groupedBatchingEnabled: false,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 				mockLogger,
 			);

diff --git a/packages/runtime/container-runtime/src/test/opLifecycle/outbox.spec.ts b/packages/runtime/container-runtime/src/test/opLifecycle/outbox.spec.ts
@@ -232,7 +232,6 @@ describe("Outbox", () => {
 				params.opGroupingConfig ?? {
 					groupedBatchingEnabled: false,
 					opCountThreshold: Infinity,
-					reentrantBatchGroupingEnabled: false,
 				},
 				mockLogger,
 			),
@@ -326,7 +325,6 @@ describe("Outbox", () => {
 			opGroupingConfig: {
 				groupedBatchingEnabled: true,
 				opCountThreshold: 2,
-				reentrantBatchGroupingEnabled: true,
 			},
 		});
 		currentSeqNumbers.referenceSequenceNumber = 0;
@@ -358,7 +356,6 @@ describe("Outbox", () => {
 			opGroupingConfig: {
 				groupedBatchingEnabled: true,
 				opCountThreshold: 3,
-				reentrantBatchGroupingEnabled: true,
 			},
 		});
 		// Flush 1 - resubmit multi-message batch including ID Allocation
@@ -1009,7 +1006,6 @@ describe("Outbox", () => {
 				opGroupingConfig: {
 					groupedBatchingEnabled: false,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 			});
 
@@ -1032,7 +1028,6 @@ describe("Outbox", () => {
 				opGroupingConfig: {
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 			});
 
@@ -1057,7 +1052,6 @@ describe("Outbox", () => {
 				opGroupingConfig: {
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 			});
 
@@ -1080,7 +1074,6 @@ describe("Outbox", () => {
 				opGroupingConfig: {
 					groupedBatchingEnabled: false,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 			});
 
@@ -1103,7 +1096,6 @@ describe("Outbox", () => {
 				opGroupingConfig: {
 					groupedBatchingEnabled: true,
 					opCountThreshold: 2,
-					reentrantBatchGroupingEnabled: true,
 				},
 			});
 

diff --git a/packages/runtime/container-runtime/src/test/opLifecycle/remoteMessageProcessor.spec.ts b/packages/runtime/container-runtime/src/test/opLifecycle/remoteMessageProcessor.spec.ts
@@ -39,7 +39,6 @@ describe("RemoteMessageProcessor", () => {
 				{
 					groupedBatchingEnabled: true,
 					opCountThreshold: Infinity,
-					reentrantBatchGroupingEnabled: false,
 				},
 				logger,
 			),
@@ -130,7 +129,6 @@ describe("RemoteMessageProcessor", () => {
 					{
 						groupedBatchingEnabled: true,
 						opCountThreshold: 2,
-						reentrantBatchGroupingEnabled: false,
 					},
 					mockLogger,
 				);