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

fix(NODE-3279): use "hello" for monitoring if supported #2895

Merged
merged 6 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
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