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.

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

fix(NODE-4608): prevent parallel monitor checks #3404

Merged
merged 6 commits into from
Oct 4, 2022
+5 −43
Merged

fix(NODE-4608): prevent parallel monitor checks #3404

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cdce4bf
test: add tests for MonitorInterval
baileympearson Sep 16, 2022
0ace76c
fix: prevent parallel monitor checks
baileympearson Sep 16, 2022
ecd4aa6
unskip sdam tests
baileympearson Oct 3, 2022
61a9a0a
Unskip flakey tests
baileympearson Oct 3, 2022
52f8e20
unskip ALL tests
baileympearson Oct 3, 2022
d0af115
fix: call callback asynchronously
baileympearson Oct 4, 2022
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
4 changes: 4 additions & 0 deletions src/sdam/monitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ function checkServer(monitor: Monitor, callback: Callback<Document | null>) {

function monitorServer(monitor: Monitor) {
return (callback: Callback) => {
if (monitor.s.state === STATE_MONITORING) {
callback();
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to use process.nextTick(callback) here

return;
}
stateTransition(monitor, STATE_MONITORING);
function done() {
if (!isInCloseState(monitor)) {
Expand Down
44 changes: 1 addition & 43 deletions .../integration/server-discovery-and-monitoring/server_discovery_and_monitoring.spec.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,8 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { Socket } from 'net';
import * as path from 'path';

import { loadSpecTests } from '../../spec';
import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
import { TestFilter } from '../../tools/unified-spec-runner/schema';
import { sleep } from '../../tools/utils';

const filter: TestFilter = ({ description }) => {
switch (description) {
case 'Network error on Monitor check':
case 'Network timeout on Monitor check':
return 'TODO(NODE-4608): Disallow parallel monitor checks';
default:
return false;
}
};

describe('SDAM Unified Tests', function () {
afterEach(async function () {
if (this.currentTest!.pending) {
return;
}
// TODO(NODE-4573): fix socket leaks
const LEAKY_TESTS = [
'Command error on Monitor handshake',
'Network error on Monitor check',
'Network timeout on Monitor check',
'Network error on Monitor handshake',
'Network timeout on Monitor handshake'
];

await sleep(250);
const sockArray = (process as any)._getActiveHandles().filter(handle => {
// Stdio are instanceof Socket so look for fd to be null
return handle.fd == null && handle instanceof Socket && handle.destroyed !== true;
});
if (!sockArray.length) {
return;
}
for (const sock of sockArray) {
sock.destroy();
}
if (!LEAKY_TESTS.some(test => test === this.currentTest!.title)) {
this.test!.error(new Error(`Test failed to clean up ${sockArray.length} socket(s)`));
}
});
runUnifiedSuite(loadSpecTests(path.join('server-discovery-and-monitoring', 'unified')), filter);
runUnifiedSuite(loadSpecTests(path.join('server-discovery-and-monitoring', 'unified')));
});