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

feat(NODE-4720): Add log messages to CMAP spec #3645

Merged
merged 37 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
8e34408
feat(NODE-4720): Add cmap log messages
W-A-James Apr 14, 2023
bd4f3b9
refactor(NODE-4720): add client close operation
W-A-James Apr 17, 2023
185dc7a
fix(NODE-4720): use debug logger
W-A-James Apr 17, 2023
3a3755c
test(NODE-4720): pull in cmap loggingt tests
W-A-James Apr 17, 2023
a6ae55e
fix(NODE-4720): start cmap debugging
W-A-James Apr 18, 2023
f7460a4
refactor(NODE-5170): update unified spec test runner
W-A-James Apr 14, 2023
e229978
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
W-A-James Apr 19, 2023
58b1f8d
refactor(NODE-4720): Fix log collector
W-A-James Apr 20, 2023
30e0a6d
WIP
W-A-James Apr 20, 2023
013c252
test(NODE-4720): Fix skip reason
W-A-James Apr 26, 2023
f5850c0
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
W-A-James Apr 26, 2023
132e16a
test(NODE-4720): sync spec tests
W-A-James Apr 26, 2023
793831f
Merge branch 'NODE-4720/add_log_messages_to_cmap_spec' of github.com:…
W-A-James Apr 26, 2023
b8d4f22
test(NODE-4720): Add modified spec test
W-A-James Apr 26, 2023
a519db5
fix(NODE-4720): Update Topology constructor
W-A-James Apr 27, 2023
8cfd0cc
test(NODE-4720): remove console.log
W-A-James Apr 27, 2023
d9dff09
style(NODE-4720): remove commented code
W-A-James Apr 27, 2023
9496a0d
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
W-A-James Apr 27, 2023
9b20b3e
Merge branch 'NODE-4720/add_log_messages_to_cmap_spec' of github.com:…
W-A-James Apr 27, 2023
c98d6a9
fix(NODE-4720): start addressing review comments
W-A-James May 2, 2023
f336b93
test(NODE-4720): test fix
W-A-James May 2, 2023
b5f3348
test(NODE-4720): undo janky fix for evg
W-A-James May 2, 2023
b62637e
fix(NODE-4720): Fix test failures
W-A-James May 2, 2023
4ace6fa
fix(NODE-4720): review comment fixes
W-A-James May 2, 2023
8c8a9f6
fix(NODE-4720): update typedEE
W-A-James May 3, 2023
62e5ea2
test(NODE-4720): fix test failure
W-A-James May 3, 2023
019913f
style(NODE-4720): eslint
W-A-James May 3, 2023
671dacc
test(NODE-4720): fix test failures
W-A-James May 3, 2023
3bcbe16
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
W-A-James May 3, 2023
b86180a
test(NODE-4720): test update
W-A-James May 3, 2023
50cb515
test(NODE-4720): move to ts
W-A-James May 4, 2023
2075178
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
W-A-James May 4, 2023
ea213bb
Merge branch 'NODE-4720/add_log_messages_to_cmap_spec' of github.com:…
W-A-James May 4, 2023
4fe466a
test(NODE-4720): test refactor
W-A-James May 4, 2023
0855d8e
style(NODE-4720): eslint
W-A-James May 4, 2023
f5f1b71
style(NODE-4720): move comment location
W-A-James May 8, 2023
5c2b31b
Merge branch 'main' into NODE-4720/add_log_messages_to_cmap_spec
nbbeeken May 8, 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
132 changes: 74 additions & 58 deletions src/cmap/connection_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
this[kProcessingWaitQueue] = false;

process.nextTick(() => {
this.emit(ConnectionPool.CONNECTION_POOL_CREATED, new ConnectionPoolCreatedEvent(this));
const connectionPoolCreatedEvent = new ConnectionPoolCreatedEvent(this);
this.emit(ConnectionPool.CONNECTION_POOL_CREATED, connectionPoolCreatedEvent);
this.client.mongoLogger.debug('connection', connectionPoolCreatedEvent);
});
}

Expand Down Expand Up @@ -322,6 +324,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
return this[kCheckedOut];
}

get client() {
return this[kServer].s.topology.client;
}
/**
* Get the metrics information for the pool when a wait queue timeout occurs.
*/
Expand All @@ -337,7 +342,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
return;
}
this[kPoolState] = PoolState.ready;
this.emit(ConnectionPool.CONNECTION_POOL_READY, new ConnectionPoolReadyEvent(this));
const connectionPoolReadyEvent = new ConnectionPoolReadyEvent(this);
this.emit(ConnectionPool.CONNECTION_POOL_READY, connectionPoolReadyEvent);
this.client.mongoLogger.debug('connection', connectionPoolReadyEvent);
clearTimeout(this[kMinPoolSizeTimer]);
this.ensureMinPoolSize();
}
Expand All @@ -348,10 +355,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
* explicitly destroyed by the new owner.
*/
checkOut(callback: Callback<Connection>): void {
this.emit(
ConnectionPool.CONNECTION_CHECK_OUT_STARTED,
new ConnectionCheckOutStartedEvent(this)
);
const connectionCheckOutStartedEvent = new ConnectionCheckOutStartedEvent(this);
this.emit(ConnectionPool.CONNECTION_CHECK_OUT_STARTED, connectionCheckOutStartedEvent);
this.client.mongoLogger.debug('connection', connectionCheckOutStartedEvent);

const waitQueueMember: WaitQueueMember = { callback };
const waitQueueTimeoutMS = this.options.waitQueueTimeoutMS;
Expand All @@ -360,10 +366,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
waitQueueMember[kCancelled] = true;
waitQueueMember.timer = undefined;

this.emit(
ConnectionPool.CONNECTION_CHECK_OUT_FAILED,
new ConnectionCheckOutFailedEvent(this, 'timeout')
);
const connectionCheckOutFailedEvent = new ConnectionCheckOutFailedEvent(this, 'timeout');
this.emit(ConnectionPool.CONNECTION_CHECK_OUT_FAILED, connectionCheckOutFailedEvent);
this.client.mongoLogger.debug('connection', connectionCheckOutFailedEvent);
waitQueueMember.callback(
new WaitQueueTimeoutError(
this.loadBalanced
Expand Down Expand Up @@ -398,7 +403,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
}

this[kCheckedOut].delete(connection);
this.emit(ConnectionPool.CONNECTION_CHECKED_IN, new ConnectionCheckedInEvent(this, connection));
const connectionCheckedInEvent = new ConnectionCheckedInEvent(this, connection);
this.emit(ConnectionPool.CONNECTION_CHECKED_IN, connectionCheckedInEvent);
this.client.mongoLogger.debug('connection', connectionCheckedInEvent);

if (willDestroy) {
const reason = connection.closed ? 'error' : poolClosed ? 'poolClosed' : 'stale';
Expand Down Expand Up @@ -437,10 +444,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
// Increment the generation for the service id.
this.serviceGenerations.set(sid, generation + 1);
}
this.emit(
ConnectionPool.CONNECTION_POOL_CLEARED,
new ConnectionPoolClearedEvent(this, { serviceId })
);
const connectionPoolClearedEvent = new ConnectionPoolClearedEvent(this, { serviceId });
this.emit(ConnectionPool.CONNECTION_POOL_CLEARED, connectionPoolClearedEvent);
this.client.mongoLogger.debug('connection', connectionPoolClearedEvent);
return;
}
// handle non load-balanced case
Expand All @@ -452,10 +458,11 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {

this.clearMinPoolSizeTimer();
if (!alreadyPaused) {
this.emit(
ConnectionPool.CONNECTION_POOL_CLEARED,
new ConnectionPoolClearedEvent(this, { interruptInUseConnections })
);
const connectionPoolClearedEvent = new ConnectionPoolClearedEvent(this, {
interruptInUseConnections
});
this.emit(ConnectionPool.CONNECTION_POOL_CLEARED, connectionPoolClearedEvent);
this.client.mongoLogger.debug('connection', connectionPoolClearedEvent);
}

if (interruptInUseConnections) {
Expand Down Expand Up @@ -509,15 +516,16 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
eachAsync<Connection>(
this[kConnections].toArray(),
(conn, cb) => {
this.emit(
ConnectionPool.CONNECTION_CLOSED,
new ConnectionClosedEvent(this, conn, 'poolClosed')
);
const connectionClosedEvent = new ConnectionClosedEvent(this, conn, 'poolClosed');
this.emit(ConnectionPool.CONNECTION_CLOSED, connectionClosedEvent);
this.client.mongoLogger.debug('connection', connectionClosedEvent);
conn.destroy({ force: !!options.force }, cb);
},
err => {
this[kConnections].clear();
this.emit(ConnectionPool.CONNECTION_POOL_CLOSED, new ConnectionPoolClosedEvent(this));
const poolClosedEvent = new ConnectionPoolClosedEvent(this);
this.emit(ConnectionPool.CONNECTION_POOL_CLOSED, poolClosedEvent);
this.client.mongoLogger.debug('connection', poolClosedEvent);
callback(err);
}
);
Expand Down Expand Up @@ -645,10 +653,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
connection: Connection,
reason: 'error' | 'idle' | 'stale' | 'poolClosed'
) {
this.emit(
ConnectionPool.CONNECTION_CLOSED,
new ConnectionClosedEvent(this, connection, reason)
);
const connectionClosedEvent = new ConnectionClosedEvent(this, connection, reason);
this.emit(ConnectionPool.CONNECTION_CLOSED, connectionClosedEvent);
this.client.mongoLogger.debug('connection', connectionClosedEvent);
// destroy the connection
process.nextTick(() => connection.destroy({ force: false }));
}
Expand Down Expand Up @@ -694,24 +701,22 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {

this[kPending]++;
// This is our version of a "virtual" no-I/O connection as the spec requires
this.emit(
ConnectionPool.CONNECTION_CREATED,
new ConnectionCreatedEvent(this, { id: connectOptions.id })
);
const connectionCreatedEvent = new ConnectionCreatedEvent(this, { id: connectOptions.id });
this.emit(ConnectionPool.CONNECTION_CREATED, connectionCreatedEvent);
this.client.mongoLogger.debug('connection', connectionCreatedEvent);

connect(connectOptions, (err, connection) => {
if (err || !connection) {
this[kPending]--;
this.emit(
ConnectionPool.CONNECTION_CLOSED,
new ConnectionClosedEvent(
this,
{ id: connectOptions.id, serviceId: undefined },
'error',
// TODO(NODE-5192): Remove this cast
err as MongoError
)
const connectionClosedEvent = new ConnectionClosedEvent(
this,
{ id: connectOptions.id, serviceId: undefined },
'error',
// TODO(NODE-5192): Remove this cast
err as MongoError
);
this.emit(ConnectionPool.CONNECTION_CLOSED, connectionClosedEvent);
this.client.mongoLogger.debug('connection', connectionClosedEvent);
if (err instanceof MongoNetworkError || err instanceof MongoServerError) {
err.connectionGeneration = connectOptions.generation;
}
Expand All @@ -729,7 +734,12 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {

// forward all events from the connection to the pool
for (const event of [...APM_EVENTS, Connection.CLUSTER_TIME_RECEIVED]) {
connection.on(event, (e: any) => this.emit(event, e));
connection.on(event, (e: any) => {
this.emit(event, e);
if (event !== Connection.CLUSTER_TIME_RECEIVED) {
this.client.mongoLogger.debug('command', event);
}
});
}

if (this.loadBalanced) {
Expand All @@ -750,7 +760,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
}

connection.markAvailable();
this.emit(ConnectionPool.CONNECTION_READY, new ConnectionReadyEvent(this, connection));
const connectionReadyEvent = new ConnectionReadyEvent(this, connection);
this.emit(ConnectionPool.CONNECTION_READY, connectionReadyEvent);
this.client.mongoLogger.debug('connection', connectionReadyEvent);

this[kPending]--;
callback(undefined, connection);
Expand Down Expand Up @@ -819,10 +831,13 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
if (this[kPoolState] !== PoolState.ready) {
const reason = this.closed ? 'poolClosed' : 'connectionError';
const error = this.closed ? new PoolClosedError(this) : new PoolClearedError(this);
this.emit(
ConnectionPool.CONNECTION_CHECK_OUT_FAILED,
new ConnectionCheckOutFailedEvent(this, reason, error)
const connectionCheckOutFailedEvent = new ConnectionCheckOutFailedEvent(
this,
reason,
error
);
this.emit(ConnectionPool.CONNECTION_CHECK_OUT_FAILED, connectionCheckOutFailedEvent);
this.client.mongoLogger.debug('connection', connectionCheckOutFailedEvent);
if (waitQueueMember.timer) {
clearTimeout(waitQueueMember.timer);
}
Expand All @@ -842,10 +857,9 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {

if (!this.destroyConnectionIfPerished(connection)) {
this[kCheckedOut].add(connection);
this.emit(
ConnectionPool.CONNECTION_CHECKED_OUT,
new ConnectionCheckedOutEvent(this, connection)
);
const connectionCheckedOutEvent = new ConnectionCheckedOutEvent(this, connection);
this.emit(ConnectionPool.CONNECTION_CHECKED_OUT, connectionCheckedOutEvent);
this.client.mongoLogger.debug('connection', connectionCheckedOutEvent);
if (waitQueueMember.timer) {
clearTimeout(waitQueueMember.timer);
}
Expand All @@ -872,17 +886,19 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
}
} else {
if (err) {
this.emit(
ConnectionPool.CONNECTION_CHECK_OUT_FAILED,
// TODO(NODE-5192): Remove this cast
new ConnectionCheckOutFailedEvent(this, 'connectionError', err as MongoError)
// TODO(NODE-5192): Remove this cast
dariakp marked this conversation as resolved.
Show resolved Hide resolved
const connectionCheckOutFailedEvent = new ConnectionCheckOutFailedEvent(
this,
'connectionError',
err as MongoError
);
this.emit(ConnectionPool.CONNECTION_CHECK_OUT_FAILED, connectionCheckOutFailedEvent);
this.client.mongoLogger.debug('connection', connectionCheckOutFailedEvent);
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
} else if (connection) {
this[kCheckedOut].add(connection);
this.emit(
ConnectionPool.CONNECTION_CHECKED_OUT,
new ConnectionCheckedOutEvent(this, connection)
);
const connectionCheckedOutEvent = new ConnectionCheckedOutEvent(this, connection);
this.emit(ConnectionPool.CONNECTION_CHECKED_OUT, connectionCheckedOutEvent);
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
this.client.mongoLogger.debug('connection', connectionCheckedOutEvent);
}

if (waitQueueMember.timer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { loadSpecTests } from '../../spec';
import { CmapTest, runCmapTestSuite, SkipDescription } from '../../tools/cmap_spec_runner';
import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';

// These tests rely on a simple "pool.clear()" command, which is not sufficient
// to properly clear the pool in LB mode, since it requires a serviceId to be passed in
Expand Down Expand Up @@ -28,15 +29,15 @@ const INTERRUPT_IN_USE_SKIPPED_TESTS: SkipDescription[] = [
},
{
description:
'Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections: false)',
'Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections = false)',
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
skipIfCondition: 'always',
skipReason:
'NodeJS does not have a background thread responsible for managing connections, and so already checked in connections are not pruned when in-use connections are interrupted.'
}
];

describe('Connection Monitoring and Pooling Spec Tests (Integration)', function () {
const tests: CmapTest[] = loadSpecTests('connection-monitoring-and-pooling');
describe('Connection Monitoring and Pooling Spec Tests (Integration) - cmap-format', function () {
const tests: CmapTest[] = loadSpecTests('connection-monitoring-and-pooling', 'cmap-format');

runCmapTestSuite(tests, {
testsToSkip: LB_SKIP_TESTS.concat(
Expand All @@ -52,3 +53,19 @@ describe('Connection Monitoring and Pooling Spec Tests (Integration)', function
)
});
});

describe('Connection Monitoring and Pooling Spec Tests (Integration) - logging', function () {
const tests = loadSpecTests('connection-monitoring-and-pooling', 'logging');

runUnifiedSuite(tests, (test: { description: string }) => {
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
if (
[
'waitQueueMultiple should be included in connection pool created message when specified',
'waitQueueSize should be included in connection pool created message when specified'
].includes(test.description)
) {
return 'not applicable: waitQueueSize not supported';
}
W-A-James marked this conversation as resolved.
Show resolved Hide resolved
return false;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"name": "waitForEvent",
"event": "ConnectionPoolCleared",
"count": 1,
"timeout": 1000
"timeout": 1000
},
{
"name": "waitForEvent",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
},
{
"type": "ConnectionCreated"
},
},
{
"type": "ConnectionPoolCleared",
"interruptInUseConnections": true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"version": 1,
"style": "unit",
"description": "Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections: false)",
"description": "Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections = false)",
"poolOptions": {
"backgroundThreadIntervalMS": 10000
},
Expand All @@ -28,7 +28,7 @@
"name": "waitForEvent",
"event": "ConnectionPoolCleared",
"count": 1,
"timeout": 1000
"timeout": 1000
},
{
"name": "waitForEvent",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
version: 1
style: unit
description: Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections: false)
description: Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections = false)
poolOptions:
# ensure it's not involved by default
backgroundThreadIntervalMS: 10000
Expand Down
Loading