Skip to content

Commit

Permalink
[2.0.0.rc-1.0] fix(directory): Fallback to previous subdirectory orde…
Browse files Browse the repository at this point in the history
…ring on mismatch (microsoft#19539)

## Description

Relaxes a consistency assert in SharedDirectory's subdirectory ordering
scheme. This was a recent feature implemented in microsoft#17816.

Production telemetry shows hits for this assert which we don't yet
understand. However, nobody yet relies on correctness of subdirectory
ordering, so falling back to previous behavior while we root cause is a
relatively safe course of action.

Since we haven't been able to reproduce this failure mode using unit
tests even after scrutiny, this also adds a telemetry log (once per
module load) with some basic statistics about the mismatch.

Cherry-pick of microsoft#19536

Co-authored-by: Abram Sanderson <[email protected]>
  • Loading branch information
Abe27342 and Abram Sanderson authored Feb 7, 2024
1 parent cfcca0d commit 9e20b3a
Showing 1 changed file with 29 additions and 5 deletions.
34 changes: 29 additions & 5 deletions packages/dds/map/src/directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { assert } from "@fluidframework/core-utils";
import { TypedEventEmitter } from "@fluid-internal/client-utils";
import { UsageError } from "@fluidframework/telemetry-utils";
import { ITelemetryLoggerExt, UsageError } from "@fluidframework/telemetry-utils";
import { readAndParse } from "@fluidframework/driver-utils";
import { ISequencedDocumentMessage, MessageType } from "@fluidframework/protocol-definitions";
import {
Expand Down Expand Up @@ -433,6 +433,10 @@ class DirectoryCreationTracker {
}, keys);
return keys;
}

public get size(): number {
return this.keyToIndex.size;
}
}

/**
Expand Down Expand Up @@ -498,6 +502,7 @@ export class SharedDirectory
this.runtime,
this.serializer,
posix.sep,
this.logger,
);

/**
Expand Down Expand Up @@ -810,6 +815,7 @@ export class SharedDirectory
this.runtime,
this.serializer,
posix.join(currentSubDir.absolutePath, subdirName),
this.logger,
);
currentSubDir.populateSubDirectory(subdirName, newSubDir);
// Record the newly inserted subdirectory to the creation tracker
Expand Down Expand Up @@ -1219,6 +1225,8 @@ function assertNonNullClientId(clientId: string | null): asserts clientId is str
assert(clientId !== null, 0x6af /* client id should never be null */);
}

let hasLoggedDirectoryInconsistency = false;

/**
* Node of the directory tree.
* @sealed
Expand Down Expand Up @@ -1309,6 +1317,7 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
private readonly runtime: IFluidDataStoreRuntime,
private readonly serializer: IFluidSerializer,
public readonly absolutePath: string,
private readonly logger: ITelemetryLoggerExt,
) {
super();
this.localCreationSeqTracker = new DirectoryCreationTracker();
Expand Down Expand Up @@ -1510,10 +1519,24 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec

const subdirNames = [...ackedSubdirsInOrder, ...localSubdirsInOrder];

assert(
subdirNames.length === this._subdirectories.size,
0x85c /* The count of keys for iteration should be consistent with the size of actual data */,
);
if (subdirNames.length !== this._subdirectories.size) {
// TODO: AB#7022: Hitting this block indicates that the eventual consistency scheme for ordering subdirectories
// has failed. Fall back to previous directory behavior, which didn't guarantee ordering.
// It's not currently clear how to reach this state, so log some diagnostics to help understand the issue.
// This whole block should eventually be replaced by an assert that the two sizes align.
if (!hasLoggedDirectoryInconsistency) {
this.logger.sendTelemetryEvent({
eventName: "inconsistentSubdirectoryOrdering",
localKeyCount: this.localCreationSeqTracker.size,
ackedKeyCount: this.ackedCreationSeqTracker.size,
subdirNamesLength: subdirNames.length,
subdirectoriesSize: this._subdirectories.size,
});
hasLoggedDirectoryInconsistency = true;
}

return this._subdirectories.entries();
}

const entriesIterator = {
index: 0,
Expand Down Expand Up @@ -2596,6 +2619,7 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
this.runtime,
this.serializer,
absolutePath,
this.logger,
);
/**
* Store the sequnce numbers of newly created subdirectory to the proper creation tracker, based
Expand Down

0 comments on commit 9e20b3a

Please sign in to comment.