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

Author: emadum

.evergreen/config.yml

@@ -1,6 +1,6 @@
 stepback: true
 command_type: system
-exec_timeout_secs: 900
+exec_timeout_secs: 1200
 timeout:
   - command: shell.exec
     params:

.evergreen/config.yml.in

@@ -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:

src/cmap/connect.ts

@@ -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);
@@ -158,6 +162,7 @@ function performInitialHandshake(
 export interface HandshakeDocument extends Document {
   ismaster?: boolean;
   hello?: boolean;
+  helloOk?: boolean;
   client: ClientMetadata;
   compression: string[];
   saslSupportedMechs?: string;
@@ -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
   };

src/cmap/connection.ts

@@ -164,6 +164,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
   destroyed: boolean;
   lastIsMasterMS?: number;
   serverApi?: ServerApi;
+  helloOk?: boolean;
   /** @internal */
   [kDescription]: StreamDescription;
   /** @internal */

src/sdam/monitor.ts

@@ -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) }
         : {})

src/sdam/server_description.ts

@@ -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;

test/functional/mongo_client_options.test.js

@@ -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);
         });
       });
@@ -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);
         });
       });

test/spec/server-discovery-and-monitoring/errors/error_handling_handshake.json

@@ -9,7 +9,8 @@
           "a:27017",
           {
             "ok": 1,
-            "ismaster": true,
+            "helloOk": true,
+            "isWritablePrimary": true,
             "hosts": [
               "a:27017"
             ],

test/spec/server-discovery-and-monitoring/errors/error_handling_handshake.yml

@@ -5,7 +5,8 @@ phases:
   responses:
   - - a:27017
     - ok: 1
-      ismaster: true
+      helloOk: true
+      isWritablePrimary: true
       hosts:
       - a:27017
       setName: rs

test/spec/server-discovery-and-monitoring/errors/generate-error-tests.py

@@ -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,),
 }
 
@@ -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]