Skip to content

feat(NODE-5189): deprecate tcp keepalive options #3621

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 13, 2023
Merged

feat(NODE-5189): deprecate tcp keepalive options #3621

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1547ce
feat(NODE-3607): update tcp keepalive options
durran Apr 4, 2023
1de8cba
Merge branch 'main' into NODE-3607
durran Apr 11, 2023
47c81d8
chore: updating per comments
durran Apr 12, 2023
11a5600
fix: remove changes outside deprecations
durran Apr 13, 2023
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
9 changes: 4 additions & 5 deletions src/cmap/connect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,13 @@ const SOCKET_ERROR_EVENTS = new Set(SOCKET_ERROR_EVENT_LIST);
function makeConnection(options: MakeConnectionOptions, _callback: Callback<Stream>) {
const useTLS = options.tls ?? false;
const keepAlive = options.keepAlive ?? true;
const socketTimeoutMS = options.socketTimeoutMS ?? Reflect.get(options, 'socketTimeout') ?? 0;
const noDelay = options.noDelay ?? true;
const connectTimeoutMS = options.connectTimeoutMS ?? 30000;
const rejectUnauthorized = options.rejectUnauthorized ?? true;
const keepAliveInitialDelay =
((options.keepAliveInitialDelay ?? 120000) > socketTimeoutMS
? Math.round(socketTimeoutMS / 2)
: options.keepAliveInitialDelay) ?? 120000;
// Default to delay to 300 seconds. Node automatically then sets TCP_KEEPINTVL to 1 second
// which is acceptable to the recommendation of 10 seconds and also cannot be configured.
// TCP_KEEPCNT is also set to 10 in Node and cannot be configured. (Recommendation is 9)
const keepAliveInitialDelay = options.keepAliveInitialDelay;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only want to do the deprecations; changing the defaults would be a potentially breaking change (at any rate it would change behavior), so we can do that in NODE-5190.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the changes, only deprecations now.

const existingSocket = options.existingSocket;

let socket: Stream;
Expand Down
2 changes: 2 additions & 0 deletions src/cmap/connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ export interface ConnectionOptions
credentials?: MongoCredentials;
connectTimeoutMS?: number;
tls: boolean;
/** @deprecated - Will not be able to turn off in the future. */
keepAlive?: boolean;
/** @deprecated - Will not be configurable in the future. */
keepAliveInitialDelay?: number;
noDelay?: boolean;
socketTimeoutMS?: number;
Expand Down
8 changes: 5 additions & 3 deletions src/connection_string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,11 +866,13 @@ export const OPTIONS = {
},
keepAlive: {
default: true,
type: 'boolean'
type: 'boolean',
deprecated: 'Will not be able to turn off in the future.'
},
keepAliveInitialDelay: {
default: 120000,
type: 'uint'
default: 300000,
type: 'uint',
deprecated: 'Will not be configurable in the future.'
},
loadBalanced: {
default: false,
Expand Down
7 changes: 5 additions & 2 deletions src/mongo_client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,12 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
sslCRL?: string;
/** TCP Connection no delay */
noDelay?: boolean;
/** TCP Connection keep alive enabled */
/** @deprecated TCP Connection keep alive enabled. Will not be able to turn off in the future. */
keepAlive?: boolean;
/** The number of milliseconds to wait before initiating keepAlive on the TCP socket */
/**
* @deprecated The number of milliseconds to wait before initiating keepAlive on the TCP socket.
* Will not be configurable in the future.
*/
keepAliveInitialDelay?: number;
/** Force server to assign `_id` values instead of driver */
forceServerObjectId?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion test/unit/mongo_client.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ describe('MongoOptions', function () {
['forceserverobjectid', false],
['heartbeatfrequencyms', 10000],
['keepalive', true],
['keepaliveinitialdelay', 120000],
['keepaliveinitialdelay', 300000],
['localthresholdms', 15],
['maxidletimems', 0],
['maxpoolsize', 100],
Expand Down