Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(NODE-6524): consolidate socket timeout calculation #4320

Merged
merged 3 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
...options
};

if (!options.omitMaxTimeMS) {
const maxTimeMS = options.timeoutContext?.maxTimeMS;
if (maxTimeMS && maxTimeMS > 0 && Number.isFinite(maxTimeMS)) cmd.maxTimeMS = maxTimeMS;
}
options.timeoutContext?.addMaxTimeMSToCommand(cmd, options);

const message = this.supportsOpMsg
? new OpMsgRequest(db, cmd, commandOptions)
Expand All @@ -446,13 +443,11 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
): AsyncGenerator<MongoDBResponse> {
this.throwIfAborted();

if (options.timeoutContext?.csotEnabled()) {
this.socket.setTimeout(0);
} else if (typeof options.socketTimeoutMS === 'number') {
this.socket.setTimeout(options.socketTimeoutMS);
} else if (this.socketTimeoutMS !== 0) {
this.socket.setTimeout(this.socketTimeoutMS);
}
const timeout =
options.socketTimeoutMS ??
options?.timeoutContext?.getSocketTimeoutMS() ??
this.socketTimeoutMS;
this.socket.setTimeout(timeout);

try {
await this.writeCommand(message, {
Expand Down Expand Up @@ -487,11 +482,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
yield document;
this.throwIfAborted();

if (typeof options.socketTimeoutMS === 'number') {
this.socket.setTimeout(options.socketTimeoutMS);
} else if (this.socketTimeoutMS !== 0) {
this.socket.setTimeout(this.socketTimeoutMS);
}
this.socket.setTimeout(timeout);
}
} finally {
this.socket.setTimeout(0);
Expand Down
2 changes: 1 addition & 1 deletion src/cmap/connection_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
}
throw error;
} finally {
if (options.timeoutContext.clearConnectionCheckoutTimeout) timeout?.clear();
timeout?.clear();
}
}

Expand Down
11 changes: 6 additions & 5 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1169,9 +1169,6 @@ export class CursorTimeoutContext extends TimeoutContext {
override get clearServerSelectionTimeout(): boolean {
return this.timeoutContext.clearServerSelectionTimeout;
}
override get clearConnectionCheckoutTimeout(): boolean {
return this.timeoutContext.clearConnectionCheckoutTimeout;
}
override get timeoutForSocketWrite(): Timeout | null {
return this.timeoutContext.timeoutForSocketWrite;
}
Expand All @@ -1190,12 +1187,16 @@ export class CursorTimeoutContext extends TimeoutContext {
override get maxTimeMS(): number | null {
return this.timeoutContext.maxTimeMS;
}

get timeoutMS(): number | null {
return this.timeoutContext.csotEnabled() ? this.timeoutContext.timeoutMS : null;
}

override refreshed(): CursorTimeoutContext {
return new CursorTimeoutContext(this.timeoutContext.refreshed(), this.owner);
}
override addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void {
this.timeoutContext.addMaxTimeMSToCommand(command, options);
}
override getSocketTimeoutMS(): number | undefined {
return this.timeoutContext.getSocketTimeoutMS();
}
}
29 changes: 23 additions & 6 deletions src/timeout.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { type Document } from 'bson';
import { clearTimeout, setTimeout } from 'timers';

import { MongoInvalidArgumentError, MongoOperationTimeoutError, MongoRuntimeError } from './error';
Expand Down Expand Up @@ -171,8 +172,6 @@ export abstract class TimeoutContext {

abstract get clearServerSelectionTimeout(): boolean;

abstract get clearConnectionCheckoutTimeout(): boolean;

abstract get timeoutForSocketWrite(): Timeout | null;

abstract get timeoutForSocketRead(): Timeout | null;
Expand All @@ -185,6 +184,10 @@ export abstract class TimeoutContext {

/** Returns a new instance of the TimeoutContext, with all timeouts refreshed and restarted. */
abstract refreshed(): TimeoutContext;

abstract addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void;

abstract getSocketTimeoutMS(): number | undefined;
}

/** @internal */
Expand All @@ -193,7 +196,6 @@ export class CSOTTimeoutContext extends TimeoutContext {
serverSelectionTimeoutMS: number;
socketTimeoutMS?: number;

clearConnectionCheckoutTimeout: boolean;
clearServerSelectionTimeout: boolean;

private _serverSelectionTimeout?: Timeout | null;
Expand All @@ -212,7 +214,6 @@ export class CSOTTimeoutContext extends TimeoutContext {
this.socketTimeoutMS = options.socketTimeoutMS;

this.clearServerSelectionTimeout = false;
this.clearConnectionCheckoutTimeout = true;
}

get maxTimeMS(): number {
Expand Down Expand Up @@ -325,19 +326,27 @@ export class CSOTTimeoutContext extends TimeoutContext {
override refreshed(): CSOTTimeoutContext {
return new CSOTTimeoutContext(this);
}

override addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void {
if (options.omitMaxTimeMS) return;
const maxTimeMS = this.remainingTimeMS - this.minRoundTripTime;
if (maxTimeMS > 0 && Number.isFinite(maxTimeMS)) command.maxTimeMS = maxTimeMS;
}

override getSocketTimeoutMS(): number | undefined {
return 0;
}
}

/** @internal */
export class LegacyTimeoutContext extends TimeoutContext {
options: LegacyTimeoutContextOptions;
clearServerSelectionTimeout: boolean;
clearConnectionCheckoutTimeout: boolean;

constructor(options: LegacyTimeoutContextOptions) {
super();
this.options = options;
this.clearServerSelectionTimeout = true;
this.clearConnectionCheckoutTimeout = true;
}

csotEnabled(): this is CSOTTimeoutContext {
Expand Down Expand Up @@ -379,4 +388,12 @@ export class LegacyTimeoutContext extends TimeoutContext {
override refreshed(): LegacyTimeoutContext {
return new LegacyTimeoutContext(this.options);
}

override addMaxTimeMSToCommand(_command: Document, _options: { omitMaxTimeMS?: boolean }): void {
// No max timeMS is added to commands in legacy timeout mode.
}

override getSocketTimeoutMS(): number | undefined {
return this.options.socketTimeoutMS;
}
}