Skip to content

Commit

Permalink
fix(NODE-3279): use "hello" for monitoring if supported (#2895)
Browse files Browse the repository at this point in the history
  • Loading branch information
emadum authored and ljhaywar committed Nov 9, 2021
1 parent cfc909e commit d3ce523
Show file tree
Hide file tree
Showing 379 changed files with 1,466 additions and 854 deletions.
2 changes: 1 addition & 1 deletion .evergreen/config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
stepback: true
command_type: system
exec_timeout_secs: 900
exec_timeout_secs: 1200
timeout:
- command: shell.exec
params:
Expand Down
2 changes: 1 addition & 1 deletion .evergreen/config.yml.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ command_type: system
# Protect ourself against rogue test case, or curl gone wild, that runs forever
# Good rule of thumb: the averageish length a task takes, times 5
# That roughly accounts for variable system performance for various buildvariants
exec_timeout_secs: 900
exec_timeout_secs: 1200

# What to do when evergreen hits the timeout (`post:` tasks are run automatically)
timeout:
Expand Down
6 changes: 6 additions & 0 deletions src/cmap/connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ function performInitialHandshake(
response.ismaster = response.isWritablePrimary;
}

if (response.helloOk) {
conn.helloOk = true;
}

const supportedServerErr = checkSupportedServer(response, options);
if (supportedServerErr) {
callback(supportedServerErr);
Expand Down Expand Up @@ -158,6 +162,7 @@ function performInitialHandshake(
export interface HandshakeDocument extends Document {
ismaster?: boolean;
hello?: boolean;
helloOk?: boolean;
client: ClientMetadata;
compression: string[];
saslSupportedMechs?: string;
Expand All @@ -170,6 +175,7 @@ function prepareHandshakeDocument(authContext: AuthContext, callback: Callback<H

const handshakeDoc: HandshakeDocument = {
[serverApi?.version ? 'hello' : 'ismaster']: true,
helloOk: true,
client: options.metadata || makeClientMetadata(options),
compression: compressors
};
Expand Down
1 change: 1 addition & 0 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
destroyed: boolean;
lastIsMasterMS?: number;
serverApi?: ServerApi;
helloOk?: boolean;
/** @internal */
[kDescription]: StreamDescription;
/** @internal */
Expand Down
4 changes: 2 additions & 2 deletions src/sdam/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,14 @@ function checkServer(monitor: Monitor, callback: Callback<Document>) {

const connection = monitor[kConnection];
if (connection && !connection.closed) {
const { serverApi } = connection;
const { serverApi, helloOk } = connection;
const connectTimeoutMS = monitor.options.connectTimeoutMS;
const maxAwaitTimeMS = monitor.options.heartbeatFrequencyMS;
const topologyVersion = monitor[kServer].description.topologyVersion;
const isAwaitable = topologyVersion != null;

const cmd = {
[serverApi?.version ? 'hello' : 'ismaster']: true,
[serverApi?.version || helloOk ? 'hello' : 'ismaster']: true,
...(isAwaitable && topologyVersion
? { maxAwaitTimeMS, topologyVersion: makeTopologyVersion(topologyVersion) }
: {})
Expand Down
2 changes: 1 addition & 1 deletion src/sdam/server_description.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export function parseServerType(ismaster?: Document): ServerType {
if (ismaster.setName) {
if (ismaster.hidden) {
return ServerType.RSOther;
} else if (ismaster.ismaster) {
} else if (ismaster.ismaster || ismaster.isWritablePrimary) {
return ServerType.RSPrimary;
} else if (ismaster.secondary) {
return ServerType.RSSecondary;
Expand Down
22 changes: 14 additions & 8 deletions test/functional/mongo_client_options.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,16 @@ describe('MongoClient Options', function () {
const args = Array.prototype.slice.call(arguments);
const ns = args[0];
const command = args[1];
const options = args[2];
if (ns.toString() === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) {
stub.restore();
const options = args[2] || {};
if (
ns.toString() === 'admin.$cmd' &&
(command.ismaster || command.hello) &&
options.exhaustAllowed
) {
expect(options).property('socketTimeoutMS').to.equal(0);
stub.restore();
client.close(done);
}

stub.wrappedMethod.apply(this, args);
});
});
Expand All @@ -90,13 +93,16 @@ describe('MongoClient Options', function () {
const args = Array.prototype.slice.call(arguments);
const ns = args[0];
const command = args[1];
const options = args[2];
if (ns.toString() === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) {
stub.restore();
const options = args[2] || {};
if (
ns.toString() === 'admin.$cmd' &&
(command.ismaster || command.hello) &&
options.exhaustAllowed
) {
expect(options).property('socketTimeoutMS').to.equal(510);
stub.restore();
client.close(done);
}

stub.wrappedMethod.apply(this, args);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ def write_test(filename, data):
ERR_CODES = {
'InterruptedAtShutdown': (11600,),
'InterruptedDueToReplStateChange': (11602,),
'NotMasterOrSecondary': (13436,),
'NotPrimaryOrSecondary': (13436,),
'PrimarySteppedDown': (189,),
'ShutdownInProgress': (91,),
'NotMaster': (10107,),
'NotMasterNoSlaveOk': (13435,),
'NotWritablePrimary': (10107,),
'NotPrimaryNoSecondaryOk': (13435,),
'LegacyNotPrimary': (10058,),
}

Expand Down Expand Up @@ -139,7 +139,7 @@ def create_stale_generation_tests():

def create_pre_42_tests():
tmp = template('pre-42.yml.template')
# All "not master"/"node is recovering" clear the pool on <4.2
# All "not writable primary"/"node is recovering" clear the pool on <4.2
for error_name in ERR_CODES:
test_name = f'pre-42-{error_name}'
error_code, = ERR_CODES[error_name]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ phases:
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"description": "Non-stale topologyVersion greater NotMasterNoSlaveOk error",
"description": "Non-stale topologyVersion greater NotPrimaryNoSecondaryOk error",
"uri": "mongodb://a/?replicaSet=rs",
"phases": [
{
Expand All @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down Expand Up @@ -51,7 +52,7 @@
}
},
{
"description": "Non-stale topologyVersion greater NotMasterNoSlaveOk error marks server Unknown",
"description": "Non-stale topologyVersion greater NotPrimaryNoSecondaryOk error marks server Unknown",
"applicationErrors": [
{
"address": "a:27017",
Expand All @@ -60,7 +61,7 @@
"type": "command",
"response": {
"ok": 0,
"errmsg": "NotMasterNoSlaveOk",
"errmsg": "NotPrimaryNoSecondaryOk",
"code": 13435,
"topologyVersion": {
"processId": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Autogenerated tests for SDAM error handling, see generate-error-tests.py
description: Non-stale topologyVersion greater NotMasterNoSlaveOk error
description: Non-stale topologyVersion greater NotPrimaryNoSecondaryOk error
uri: mongodb://a/?replicaSet=rs
phases:
- description: Primary A is discovered
responses:
- - a:27017
- ok: 1
ismaster: true
helloOk: true
isWritablePrimary: true
hosts:
- a:27017
setName: rs
Expand All @@ -29,15 +30,15 @@ phases:
logicalSessionTimeoutMinutes: null
setName: rs

- description: Non-stale topologyVersion greater NotMasterNoSlaveOk error marks server Unknown
- description: Non-stale topologyVersion greater NotPrimaryNoSecondaryOk error marks server Unknown
applicationErrors:
- address: a:27017
when: afterHandshakeCompletes
maxWireVersion: 9
type: command
response:
ok: 0
errmsg: NotMasterNoSlaveOk
errmsg: NotPrimaryNoSecondaryOk
code: 13435
topologyVersion:
processId:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"description": "Non-stale topologyVersion greater NotMasterOrSecondary error",
"description": "Non-stale topologyVersion greater NotPrimaryOrSecondary error",
"uri": "mongodb://a/?replicaSet=rs",
"phases": [
{
Expand All @@ -9,7 +9,8 @@
"a:27017",
{
"ok": 1,
"ismaster": true,
"helloOk": true,
"isWritablePrimary": true,
"hosts": [
"a:27017"
],
Expand Down Expand Up @@ -51,7 +52,7 @@
}
},
{
"description": "Non-stale topologyVersion greater NotMasterOrSecondary error marks server Unknown",
"description": "Non-stale topologyVersion greater NotPrimaryOrSecondary error marks server Unknown",
"applicationErrors": [
{
"address": "a:27017",
Expand All @@ -60,7 +61,7 @@
"type": "command",
"response": {
"ok": 0,
"errmsg": "NotMasterOrSecondary",
"errmsg": "NotPrimaryOrSecondary",
"code": 13436,
"topologyVersion": {
"processId": {
Expand Down
Loading

0 comments on commit d3ce523

Please sign in to comment.