Skip to content

Commit

Permalink
refactor(container-runtime): Enable eslint rules from recommended c…
Browse files Browse the repository at this point in the history
…onfig and fix violations (microsoft#23564)

Adds the `jsdoc`-related rules from the recommended config and fixes
violations. Specifically,
- `jsdoc/multiline-blocks`
- `jsdoc/require-description`,

This PR is one piece of a multi-PR process to migrate the
`container-runtime` package to our recommended eslint config.


[AB#3027](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3027)
  • Loading branch information
Josmithr authored Jan 16, 2025
1 parent 831c020 commit 52a40cb
Show file tree
Hide file tree
Showing 46 changed files with 1,115 additions and 378 deletions.
2 changes: 1 addition & 1 deletion common/build/eslint-config-fluid/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module.exports = {
"unicorn/empty-brace-spaces": "off",

// Rationale: Destructuring of `Array.entries()` in order to get the index variable results in a
// significant performance regression [node 14 x64].
// significant performance regression [node 14 x64].
"unicorn/no-for-loop": "off",

/**
Expand Down
7 changes: 7 additions & 0 deletions packages/runtime/container-runtime/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ module.exports = {
rules: {
"@typescript-eslint/strict-boolean-expressions": "off",
"@fluid-internal/fluid/no-unchecked-record-access": "warn",

// #region TODO:AB#3027: remove overrides and upgrade config to `recommended`

"jsdoc/multiline-blocks": ["error", { noSingleLineBlocks: true }],
"jsdoc/require-description": ["error", { checkConstructors: false }],

// #endregion
},
overrides: [
{
Expand Down
23 changes: 17 additions & 6 deletions packages/runtime/container-runtime/src/channelCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,18 @@ import {
* @internal
*/
export enum RuntimeHeaders {
/** True to wait for a data store to be created and loaded before returning it. */
/**
* True to wait for a data store to be created and loaded before returning it.
*/
wait = "wait",
/** True if the request is coming from an IFluidHandle. */
/**
* True if the request is coming from an IFluidHandle.
*/
viaHandle = "viaHandle",
}

/** True if a tombstoned object should be returned without erroring
/**
* True if a tombstoned object should be returned without erroring
* @legacy
* @alpha
*/
Expand Down Expand Up @@ -373,7 +378,9 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
return pendingAliasPromise ?? "Success";
}

/** For sampling. Only log once per container */
/**
* For sampling. Only log once per container
*/
private shouldSendAttachLog = true;

protected wrapContextForInnerChannel(id: string): IFluidParentContext {
Expand Down Expand Up @@ -545,7 +552,9 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
return this.aliasMap.get(id) !== undefined || this.contexts.get(id) !== undefined;
}

/** Package up the context's attach summary etc into an IAttachMessage */
/**
* Package up the context's attach summary etc into an IAttachMessage
*/
private generateAttachMessage(localContext: LocalFluidDataStoreContext): IAttachMessage {
// Get the attach summary.
const attachSummary = localContext.getAttachSummary();
Expand Down Expand Up @@ -1611,7 +1620,9 @@ export function detectOutboundReferences(
outboundPaths.forEach((toPath) => addedOutboundReference(fromPath, toPath));
}

/** @internal */
/**
* @internal
*/
export class ChannelCollectionFactory<T extends ChannelCollection = ChannelCollection>
implements IFluidDataStoreFactory
{
Expand Down
36 changes: 27 additions & 9 deletions packages/runtime/container-runtime/src/connectionTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,39 @@ export const latencyThreshold = 5000;
// 3. Op received from service back (pushed to inbound queue).
// 4. Op is processed.
interface IOpPerfTelemetryProperties {
/** Measure time between (1) and (2) - Measure time outbound op is sitting in queue due to active batch */
/**
* Measure time between (1) and (2) - Measure time outbound op is sitting in queue due to active batch
*/
durationOutboundBatching: number; // was durationOutboundQueue in previous versions
/** Measure time between (2) and (3) - Track how long it took for op to be acked by service */
/**
* Measure time between (2) and (3) - Track how long it took for op to be acked by service
*/
durationNetwork: number; // was durationInboundQueue
/** Measure time between (3) and (4) - Time between DM's inbound "push" event until DM's "op" event */
/**
* Measure time between (3) and (4) - Time between DM's inbound "push" event until DM's "op" event
*/
durationInboundToProcessing: number;
/** Length of the DeltaManager's inbound queue at the time of the DM's inbound "push" event (3) */
/**
* Length of the DeltaManager's inbound queue at the time of the DM's inbound "push" event (3)
*/
lengthInboundQueue: number;
}

/**
* Timings collected at various moments during the op processing.
*/
interface IOpPerfTimings {
/** Starting time for (1) */
/**
* Starting time for (1)
*/
submitOpEventTime: number;
/** Starting time for (2) */
/**
* Starting time for (2)
*/
outboundPushEventTime: number;
/** Starting time for (3) */
/**
* Starting time for (3)
*/
inboundPushEventTime: number;
}

Expand All @@ -81,9 +95,13 @@ class OpPerfTelemetry {
private connectionStartTime = 0;
private gap = 0;

/** Count of no-ops sent by this client. This variable is reset everytime the OpStats sampled event is logged */
/**
* Count of no-ops sent by this client. This variable is reset everytime the OpStats sampled event is logged
*/
private noOpCountForTelemetry = 0;
/** Cumulative size of the ops processed by this client. This variable is reset everytime the OpStats sampled event is logged */
/**
* Cumulative size of the ops processed by this client. This variable is reset everytime the OpStats sampled event is logged
*/
private processedOpSizeForTelemetry = 0;

private readonly logger: ITelemetryLoggerExt;
Expand Down
72 changes: 54 additions & 18 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,9 @@ export const DefaultSummaryConfiguration: ISummaryConfiguration = {
* @alpha
*/
export interface ISummaryRuntimeOptions {
/** Override summary configurations set by the server. */
/**
* Override summary configurations set by the server.
*/
summaryConfigOverrides?: ISummaryConfiguration;

/**
Expand Down Expand Up @@ -584,7 +586,9 @@ export interface RuntimeHeaderData {
allowTombstone?: boolean;
}

/** Default values for Runtime Headers */
/**
* Default values for Runtime Headers
*/
export const defaultRuntimeHeaderData: Required<RuntimeHeaderData> = {
wait: true,
viaHandle: false,
Expand Down Expand Up @@ -663,9 +667,13 @@ const defaultCompressionConfig = {

const defaultChunkSizeInBytes = 204800;

/** The default time to wait for pending ops to be processed during summarization */
/**
* The default time to wait for pending ops to be processed during summarization
*/
export const defaultPendingOpsWaitTimeoutMs = 1000;
/** The default time to delay a summarization retry attempt when there are pending ops */
/**
* The default time to delay a summarization retry attempt when there are pending ops
*/
export const defaultPendingOpsRetryDelayMs = 1000;

/**
Expand Down Expand Up @@ -890,7 +898,9 @@ export class ContainerRuntime
runtimeOptions?: IContainerRuntimeOptions; // May also include options from IContainerRuntimeOptionsInternal
containerScope?: FluidObject;
containerRuntimeCtor?: typeof ContainerRuntime;
/** @deprecated Will be removed once Loader LTS version is "2.0.0-internal.7.0.0". Migrate all usage of IFluidRouter to the "entryPoint" pattern. Refer to Removing-IFluidRouter.md */
/**
* @deprecated Will be removed once Loader LTS version is "2.0.0-internal.7.0.0". Migrate all usage of IFluidRouter to the "entryPoint" pattern. Refer to Removing-IFluidRouter.md
*/
requestHandler?: (request: IRequest, runtime: IContainerRuntime) => Promise<IResponse>;
provideEntryPoint: (containerRuntime: IContainerRuntime) => Promise<FluidObject>;
}): Promise<ContainerRuntime> {
Expand Down Expand Up @@ -1365,7 +1375,9 @@ export class ContainerRuntime
return this._connected;
}

/** clientId of parent (non-summarizing) container that owns summarizer container */
/**
* clientId of parent (non-summarizing) container that owns summarizer container
*/
public get summarizerClientId(): string | undefined {
return this.summarizerClientElection?.electedClientId;
}
Expand Down Expand Up @@ -1410,7 +1422,9 @@ export class ContainerRuntime
private readonly channelCollection: ChannelCollection;
private readonly remoteMessageProcessor: RemoteMessageProcessor;

/** The last message processed at the time of the last summary. */
/**
* The last message processed at the time of the last summary.
*/
private messageAtLastSummary: ISummaryMetadataMessage | undefined;

private get summarizer(): Summarizer {
Expand Down Expand Up @@ -2420,7 +2434,9 @@ export class ContainerRuntime
return this.channelCollection.internalId(maybeAlias);
}

/** Adds the container's metadata to the given summary tree. */
/**
* Adds the container's metadata to the given summary tree.
*/
private addMetadataToSummary(summaryTree: ISummaryTreeWithStats) {
// The last message processed at the time of summary. If there are no new messages, use the message from the
// last summary.
Expand Down Expand Up @@ -3664,17 +3680,29 @@ export class ContainerRuntime
* Returns a summary of the runtime at the current sequence number.
*/
public async summarize(options: {
/** True to generate the full tree with no handle reuse optimizations; defaults to false */
/**
* True to generate the full tree with no handle reuse optimizations; defaults to false
*/
fullTree?: boolean;
/** True to track the state for this summary in the SummarizerNodes; defaults to true */
/**
* True to track the state for this summary in the SummarizerNodes; defaults to true
*/
trackState?: boolean;
/** Logger to use for correlated summary events */
/**
* Logger to use for correlated summary events
*/
summaryLogger?: ITelemetryLoggerExt;
/** True to run garbage collection before summarizing; defaults to true */
/**
* True to run garbage collection before summarizing; defaults to true
*/
runGC?: boolean;
/** True to generate full GC data */
/**
* True to generate full GC data
*/
fullGC?: boolean;
/** True to run GC sweep phase after the mark phase */
/**
* True to run GC sweep phase after the mark phase
*/
runSweep?: boolean;
}): Promise<ISummaryTreeWithStats> {
this.verifyNotClosed();
Expand Down Expand Up @@ -3853,11 +3881,17 @@ export class ContainerRuntime
*/
public async collectGarbage(
options: {
/** Logger to use for logging GC events */
/**
* Logger to use for logging GC events
*/
logger?: ITelemetryLoggerExt;
/** True to run GC sweep phase after the mark phase */
/**
* True to run GC sweep phase after the mark phase
*/
runSweep?: boolean;
/** True to generate full GC data */
/**
* True to generate full GC data
*/
fullGC?: boolean;
},
telemetryContext?: ITelemetryContext,
Expand Down Expand Up @@ -4647,7 +4681,9 @@ export class ContainerRuntime
}
}

/** Implementation of ISummarizerInternalsProvider.refreshLatestSummaryAck */
/**
* Implementation of ISummarizerInternalsProvider.refreshLatestSummaryAck
*/
public async refreshLatestSummaryAck(options: IRefreshSummaryAckOptions) {
const { proposalHandle, ackHandle, summaryRefSeq, summaryLogger } = options;
// proposalHandle is always passed from RunningSummarizer.
Expand Down
8 changes: 6 additions & 2 deletions packages/runtime/container-runtime/src/dataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ import { ContainerMessageType } from "./messageTypes.js";
* alias to a datastore
*/
export interface IDataStoreAliasMessage {
/** The internal id of the datastore */
/**
* The internal id of the datastore
*/
readonly internalId: string;
/** The alias name to be assigned to the datastore */
/**
* The alias name to be assigned to the datastore
*/
readonly alias: string;
}

Expand Down
Loading

0 comments on commit 52a40cb

Please sign in to comment.