Skip to content

Commit b00591c

Browse files
authored
improvement(id-compressor): Add resetUnfinalizedCreationRange API and use in CR.replayPendingStates (#26784)
We are introducing `IdCompressor.releaseUnfinalizedCreationRange`. It's similar to `takeUnfinalizedCreationRange`, but instead of returning the range, it merely resets the internal state `nextRangeBaseGenCount` to be before any unfinalized ranges, since we know they won't be finalilzed (since the connection the ID Allocations were sent on closed without those acks). Now, the next call to `takeNextCreationRange` will include those unfinalized ranges, and we don't need to interject an extra op before replay. `takeUnfinalizedCreationRange` itself becomes a two-line shortcut.
1 parent 598ca5b commit b00591c

File tree

9 files changed

+254
-54
lines changed

9 files changed

+254
-54
lines changed

packages/runtime/container-runtime/src/containerRuntime.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,12 +2710,10 @@ export class ContainerRuntime
27102710
this.emitDirtyDocumentEvent = false;
27112711

27122712
try {
2713-
// Any ID Allocation ops that failed to submit after the pending state was queued need to have
2714-
// the corresponding ranges resubmitted (note this call replaces the typical resubmit flow).
2715-
// Since we don't submit ID Allocation ops when staged, any outstanding ranges would be from
2716-
// before staging mode so we can simply say staged: false.
2717-
this.submitIdAllocationOpIfNeeded({ resubmitOutstandingRanges: true, staged: false });
2718-
this.scheduleFlush();
2713+
// Any ID Allocation ops that failed to submit need to have their ranges included
2714+
// in the next allocation op. Reset the compressor's unfinalized range cursor so that the next
2715+
// call to takeNextCreationRange (during replay) will include those unfinalized ranges.
2716+
this._idCompressor?.resetUnfinalizedCreationRange();
27192717

27202718
// replay the ops
27212719
this.pendingStateManager.replayPendingStates();

packages/runtime/container-runtime/src/test/containerRuntime.spec.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -614,8 +614,8 @@ describe("Runtime", () => {
614614
);
615615
});
616616

617-
it("IdAllocation op from replayPendingStates is flushed, preventing outboxSequenceNumberCoherencyCheck error", async () => {
618-
// Start out disconnected since step 1 is to trigger ID Allocation op on reconnect
617+
it("IDs unfinalized due to disconnect are properly finalized after reconnect", async () => {
618+
// Start out disconnected since step 1 is to generate IDs before reconnect
619619
const connected = false;
620620
const mockContext = getMockContext({ connected }) as IContainerContext;
621621
const mockDeltaManager = mockContext.deltaManager as MockDeltaManager;
@@ -628,33 +628,49 @@ describe("Runtime", () => {
628628
provideEntryPoint: mockProvideEntryPoint,
629629
});
630630

631-
// 1st compressed id – queued while disconnected (goes to idAllocationBatch).
632-
containerRuntime.idCompressor?.generateCompressedId();
631+
const compressor = containerRuntime.idCompressor;
632+
assert(compressor !== undefined, "Expected idCompressor to be defined");
633+
634+
// Generate an ID while disconnected, and take the creation range,
635+
// but do not submit the op to leave the range unfinalized (same as if the op were submitted but not sequenced).
636+
const id1 = compressor.generateCompressedId();
637+
compressor.takeNextCreationRange();
633638

634-
// Re-connect – replayPendingStates will submit only an idAllocation op.
635-
// It's now in the Outbox and a flush is scheduled (including this flush was a bug fix)
639+
// Re-connect – replayPendingStates releases unfinalized ranges
640+
// (no IdAllocation op is submitted at this point, but the next will contain the unfinalized range).
636641
changeConnectionState(containerRuntime, true, mockClientId);
637642

638643
// Simulate a remote op arriving before we submit anything else.
639-
// Bump refSeq and continue execution at the end of the microtask queue.
640-
// This is how Inbound Queue works, and this is necessary to simulate here to allow scheduled flush to happen
644+
// Bump refSeq, emit the "op" event, and continue execution at the end of the microtask queue.
645+
// This is how Inbound Queue works, and it makes sure we get coverage of ref seq coherency in this test.
641646
++mockDeltaManager.lastSequenceNumber;
647+
mockDeltaManager.emit("op", {});
642648
await Promise.resolve();
643649

644-
// 2nd compressed id – its idAllocation op will enter Outbox *after* the ref seq# bumped.
645-
const id2 = containerRuntime.idCompressor?.generateCompressedId();
646-
647-
// This would throw a DataProcessingError from codepath "outboxSequenceNumberCoherencyCheck"
648-
// if we didn't schedule a flush after the idAllocation op submitted during the reconnect.
649-
// (On account of the two ID Allocation ops having different refSeqs but being in the same batch)
650+
// Generate another ID and submit a data store op referencing it.
651+
// This triggers submitIdAllocationOpIfNeeded → takeNextCreationRange.
652+
// Because the unfinalized range was released during replay, both IDs
653+
// are included in the resulting allocation range.
654+
const id2 = compressor.generateCompressedId();
650655
submitDataStoreOp(containerRuntime, "someDS", genTestDataStoreMessage({ id: id2 }));
651656

652657
// Let the Outbox flush so we can check submittedOps length
653658
await Promise.resolve();
654-
assert(
655-
submittedOps.length === 3,
656-
"Expected 3 ops to be submitted (2 ID Allocation, 1 data)",
657-
);
659+
assert.strictEqual(submittedOps.length, 2, "Expected 1 ID Allocation + 1 data op");
660+
661+
// Simulate processing the ID Allocation ack — finalize the range.
662+
// Without the call to resetUnfinalizedCreationRange in replayPendingStates,
663+
// this results in a "Ranges finalized out of order" error.
664+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access
665+
compressor.finalizeCreationRange(submittedOps[0].contents);
666+
667+
// Both IDs should now be finalized (positive final IDs in op space).
668+
// Without resetUnfinalizedCreationRange, id1 would remain a local-only
669+
// ID (negative) that could never be shared with other clients.
670+
const id1OpSpace = compressor.normalizeToOpSpace(id1);
671+
const id2OpSpace = compressor.normalizeToOpSpace(id2);
672+
assert(id1OpSpace >= 0, "id1 should be finalized after allocation range is sequenced");
673+
assert(id2OpSpace >= 0, "id2 should be finalized after allocation range is sequenced");
658674
});
659675
});
660676

packages/runtime/id-compressor/api-report/id-compressor.legacy.beta.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export interface IIdCompressor {
4848
export interface IIdCompressorCore {
4949
beginGhostSession(ghostSessionId: SessionId, ghostSessionCallback: () => void): void;
5050
finalizeCreationRange(range: IdCreationRange): void;
51+
resetUnfinalizedCreationRange(): void;
5152
serialize(withSession: true): SerializedIdCompressorWithOngoingSession;
5253
serialize(withSession: false): SerializedIdCompressorWithNoSession;
5354
takeNextCreationRange(): IdCreationRange;

packages/runtime/id-compressor/src/idCompressor.ts

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,12 @@ export class IdCompressor implements IIdCompressor, IIdCompressorCore {
9494

9595
// #region Final state
9696

97-
// The gen count to be annotated on the range returned by the next call to `takeNextCreationRange`.
98-
// This is updated to be equal to `generatedIdCount` + 1 each time it is called.
97+
/**
98+
* The gen count to be annotated on the range returned by the next call to `takeNextCreationRange`.
99+
* This is advanced to `generatedIdCount` + 1 each time it is called.
100+
* On the other hand, when `resetUnfinalizedCreationRange` is called,
101+
* this is moved back to the start of the unfinalized range, to ensure those IDs are included in the next range.
102+
*/
99103
private nextRangeBaseGenCount = 1;
100104
private readonly sessions = new Sessions();
101105
private readonly finalSpace = new FinalSpace();
@@ -244,35 +248,23 @@ export class IdCompressor implements IIdCompressor, IIdCompressorCore {
244248
}
245249

246250
public takeUnfinalizedCreationRange(): IdCreationRange {
247-
const lastLocalCluster = this.localSession.getLastCluster();
248-
let count: number;
249-
let firstGenCount: number;
250-
if (lastLocalCluster === undefined) {
251-
firstGenCount = 1;
252-
count = this.localGenCount;
253-
} else {
254-
firstGenCount = genCountFromLocalId(
255-
(lastLocalCluster.baseLocalId - lastLocalCluster.count) as LocalCompressedId,
256-
);
257-
count = this.localGenCount - firstGenCount + 1;
258-
}
251+
this.resetUnfinalizedCreationRange();
252+
return this.takeNextCreationRange();
253+
}
259254

260-
if (count === 0) {
261-
return {
262-
sessionId: this.localSessionId,
263-
};
264-
}
255+
public resetUnfinalizedCreationRange(): void {
256+
assert(
257+
!this.ongoingGhostSession,
258+
"IdCompressor should not be operated normally when in a ghost session",
259+
);
265260

266-
const range: IdCreationRange = {
267-
ids: {
268-
count,
269-
firstGenCount,
270-
localIdRanges: this.normalizer.getRangesBetween(firstGenCount, this.localGenCount),
271-
requestedClusterSize: this.nextRequestedClusterSize,
272-
},
273-
sessionId: this.localSessionId,
274-
};
275-
return this.updateToRange(range);
261+
const lastLocalCluster = this.localSession.getLastCluster();
262+
this.nextRangeBaseGenCount =
263+
lastLocalCluster === undefined
264+
? 1
265+
: genCountFromLocalId(
266+
(lastLocalCluster.baseLocalId - lastLocalCluster.count) as LocalCompressedId,
267+
);
276268
}
277269

278270
private updateToRange(range: IdCreationRange): IdCreationRange {

packages/runtime/id-compressor/src/test/idCompressor.spec.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,95 @@ describe("IdCompressor", () => {
461461
);
462462
});
463463
});
464+
465+
describe("by reserving unfinalized ranges for the next take", () => {
466+
it("produces equivalent range to takeUnfinalizedCreationRange on next takeNextCreationRange", () => {
467+
const compressor = CompressorFactory.createCompressor(Client.Client1, 2);
468+
generateCompressedIds(compressor, 1);
469+
compressor.takeNextCreationRange();
470+
471+
// Reserve instead of take
472+
compressor.resetUnfinalizedCreationRange();
473+
474+
// Next takeNextCreationRange should cover the unfinalized IDs
475+
const range = compressor.takeNextCreationRange();
476+
assert.deepEqual(range.ids, {
477+
firstGenCount: 1,
478+
count: 1,
479+
localIdRanges: [[1, 1]],
480+
requestedClusterSize: 2,
481+
});
482+
});
483+
484+
it("includes new IDs generated after reserving", () => {
485+
const compressor = CompressorFactory.createCompressor(Client.Client1, 2);
486+
generateCompressedIds(compressor, 1);
487+
compressor.takeNextCreationRange();
488+
489+
compressor.resetUnfinalizedCreationRange();
490+
generateCompressedIds(compressor, 1);
491+
492+
const range = compressor.takeNextCreationRange();
493+
assert.deepEqual(range.ids, {
494+
firstGenCount: 1,
495+
count: 2,
496+
localIdRanges: [[1, 2]],
497+
requestedClusterSize: 2,
498+
});
499+
});
500+
501+
it("is a no-op when there are no unfinalized IDs", () => {
502+
const compressor = CompressorFactory.createCompressor(Client.Client1, 2);
503+
generateCompressedIds(compressor, 1);
504+
compressor.finalizeCreationRange(compressor.takeNextCreationRange());
505+
506+
compressor.resetUnfinalizedCreationRange();
507+
const range = compressor.takeNextCreationRange();
508+
assert.equal(range.ids, undefined);
509+
});
510+
511+
it("is idempotent", () => {
512+
const compressor = CompressorFactory.createCompressor(Client.Client1, 2);
513+
generateCompressedIds(compressor, 2);
514+
compressor.takeNextCreationRange();
515+
516+
compressor.resetUnfinalizedCreationRange();
517+
compressor.resetUnfinalizedCreationRange();
518+
compressor.resetUnfinalizedCreationRange();
519+
520+
const range = compressor.takeNextCreationRange();
521+
assert.deepEqual(range.ids, {
522+
firstGenCount: 1,
523+
count: 2,
524+
localIdRanges: [[1, 2]],
525+
requestedClusterSize: 2,
526+
});
527+
});
528+
529+
it("works with multiple outstanding ranges", () => {
530+
const compressor = CompressorFactory.createCompressor(Client.Client1, 2);
531+
generateCompressedIds(compressor, 1);
532+
const range1 = compressor.takeNextCreationRange();
533+
generateCompressedIds(compressor, 1); // one local
534+
compressor.finalizeCreationRange(range1);
535+
compressor.takeNextCreationRange();
536+
generateCompressedIds(compressor, 1); // one eager final
537+
compressor.takeNextCreationRange();
538+
generateCompressedIds(compressor, 1); // one local
539+
compressor.takeNextCreationRange();
540+
541+
compressor.resetUnfinalizedCreationRange();
542+
const range = compressor.takeNextCreationRange();
543+
assert.deepEqual(range.ids?.firstGenCount, 2);
544+
assert.deepEqual(range.ids?.count, 3);
545+
assert.deepEqual(range.ids?.localIdRanges, [
546+
[2, 1],
547+
[4, 1],
548+
]);
549+
550+
compressor.finalizeCreationRange(range);
551+
});
552+
});
464553
});
465554

466555
describe("Finalizing", () => {

packages/runtime/id-compressor/src/types/idCompressor.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,22 @@ export interface IIdCompressorCore {
9797
*/
9898
takeUnfinalizedCreationRange(): IdCreationRange;
9999

100+
/**
101+
* Resets the next creation range to include all unfinalized IDs.
102+
*
103+
* @remarks
104+
* IMPORTANT: This must only be called if it's CERTAIN that the unfinalized range will never be finalized as-is (e.g. by in-flight ops).
105+
*
106+
* After calling this, the next call to {@link IIdCompressorCore.takeNextCreationRange} will produce a range
107+
* covering all unfinalized IDs (equivalent to what {@link IIdCompressorCore.takeUnfinalizedCreationRange} would
108+
* have returned) plus any IDs generated after this call.
109+
*
110+
* Unlike {@link IIdCompressorCore.takeUnfinalizedCreationRange}, this method does not produce or return a range,
111+
* and does not advance the internal range counter. It is useful when the caller wants to
112+
* defer the actual range submission to the next natural {@link IIdCompressorCore.takeNextCreationRange} call.
113+
*/
114+
resetUnfinalizedCreationRange(): void;
115+
100116
/**
101117
* Finalizes the supplied range of IDs (which may be from either a remote or local session).
102118
* @param range - the range of session-local IDs to finalize.

packages/runtime/test-runtime-utils/package.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@
154154
"typescript": "~5.4.5"
155155
},
156156
"typeValidation": {
157-
"broken": {},
157+
"broken": {
158+
"Class_MockFluidDataStoreContext": {
159+
"forwardCompat": false
160+
}
161+
},
158162
"entrypoint": "legacy"
159163
}
160164
}

packages/runtime/test-runtime-utils/src/test/types/validateTestRuntimeUtilsPrevious.generated.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ declare type current_as_old_for_Class_MockDeltaQueue = requireAssignableTo<TypeO
168168
* typeValidation.broken:
169169
* "Class_MockFluidDataStoreContext": {"forwardCompat": false}
170170
*/
171+
// @ts-expect-error compatibility expected to be broken
171172
declare type old_as_current_for_Class_MockFluidDataStoreContext = requireAssignableTo<TypeOnly<old.MockFluidDataStoreContext>, TypeOnly<current.MockFluidDataStoreContext>>
172173

173174
/*

packages/test/local-server-tests/src/test/opsOnReconnect.spec.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,89 @@ describe("Ops on Reconnect", () => {
260260
"Did not receive the ops that were sent in Nack'd state",
261261
);
262262
});
263+
264+
it("ID Allocations properly finalized across reconnect", async () => {
265+
// Setup with idCompressor enabled
266+
await setupFirstContainer({ enableRuntimeIdCompressor: "on" });
267+
await setupSecondContainersDataObject();
268+
269+
const compressor1 = container1Object1.context.idCompressor;
270+
assert(compressor1 !== undefined, "IdCompressor must be enabled");
271+
272+
// Capture any container-closing error so we can surface it as the test failure.
273+
// Without resetUnfinalizedCreationRange, finalizeCreationRange throws
274+
// "Ranges finalized out of order" during op processing, which closes the
275+
// container. ensureSynchronized filters out closed containers and returns
276+
// normally, so we need to re-throw the closing error ourselves.
277+
let containerCloseError: unknown;
278+
container1.on("closed", (error) => {
279+
containerCloseError = error;
280+
});
281+
282+
// Round-trip a first ID so the compressor has a finalized cluster.
283+
// This ensures the failure path exercises the contiguity check
284+
// (lastCluster.baseLocalId - lastCluster.count !== rangeBaseLocal)
285+
// rather than the first-cluster check (rangeBaseLocal !== -1).
286+
const id1 = compressor1.generateCompressedId();
287+
container1Object1Map1.set("first-op", id1);
288+
await loaderContainerTracker.ensureSynchronized();
289+
290+
// Generate a second ID and submit a DDS op while connected.
291+
// submit() calls submitIdAllocationOpIfNeeded -> takeNextCreationRange,
292+
// which advances nextRangeBaseGenCount past id2's range and queues the
293+
// IdAllocation + DDS op in the outbox batch managers. Because the flush
294+
// mode is TurnBased, the actual flush is deferred to a microtask.
295+
const id2 = compressor1.generateCompressedId();
296+
container1Object1Map1.set("pre-disconnect", id2);
297+
298+
// Disconnect synchronously, before the TurnBased microtask flush fires.
299+
// The IdAllocation and DDS ops are still sitting in the outbox.
300+
assert(container1.clientId);
301+
documentServiceFactory.disconnectClient(container1.clientId, "Disconnected for testing");
302+
assert.equal(container1.connectionState, ConnectionState.Disconnected);
303+
304+
// Reconnect. During setConnectionStateCore:
305+
// 1. flush() moves the outbox batches (including the IdAllocation
306+
// created at submit time) into PendingStateManager.
307+
// canSendOps is still false so nothing goes to the server.
308+
// 2. canSendOps becomes true.
309+
// 3. replayPendingStates() replays from PendingStateManager:
310+
// - resetUnfinalizedCreationRange is called, which releases the range for id2
311+
// back to the compressor to be included in the next creation range.
312+
// - IdAllocation ops are skipped during resubmit (by design).
313+
// - The DDS op is resubmitted, which calls submitIdAllocationOpIfNeeded
314+
// -> takeNextCreationRange. This range includes id2 (even though id2 itself may be unused).
315+
// NOTE: Without resetUnfinalizedCreationRange, nextRangeBaseGenCount is already
316+
// past id2 so count=0 and NO IdAllocation is emitted for the replayed batch.
317+
await waitForContainerConnection(container1);
318+
319+
// Generate a third ID after reconnect and submit a DDS op.
320+
// takeNextCreationRange produces a range starting at genCount 3. When the server
321+
// sequences this IdAllocation, finalizeCreationRange checks that ranges are
322+
// contiguous. Without resetUnfinalizedCreationRange the genCount-2 range
323+
// was never re-sent, so finalizing the genCount-3 range throws
324+
// "Ranges finalized out of order".
325+
const id3 = compressor1.generateCompressedId();
326+
container1Object1Map1.set("post-reconnect", id3);
327+
328+
await loaderContainerTracker.ensureSynchronized();
329+
330+
// If the container closed during op processing, surface its error as the
331+
// test failure. This is what makes the test fail with "Ranges finalized
332+
// out of order" when resetUnfinalizedCreationRange is commented out.
333+
if (containerCloseError !== undefined) {
334+
throw containerCloseError as Error;
335+
}
336+
assert(!container1.closed, "Container should not have closed");
337+
338+
// Verify all IDs are finalized and usable.
339+
const opSpaceId1 = compressor1.normalizeToOpSpace(id1);
340+
assert(opSpaceId1 >= 0, "First ID should be finalized");
341+
const opSpaceId2 = compressor1.normalizeToOpSpace(id2);
342+
assert(opSpaceId2 >= 0, "Second ID should be finalized after reconnect");
343+
const opSpaceId3 = compressor1.normalizeToOpSpace(id3);
344+
assert(opSpaceId3 >= 0, "Third ID should be finalized after reconnect");
345+
});
263346
});
264347

265348
describe("Ordering of ops that are sent in disconnected state", () => {

0 commit comments

Comments
 (0)