fix: update server description when equal #2256

emadum · 2020-03-09T19:55:28Z

Description

NODE-2474 - Server_Description update with lastUpdateTime / lastWriteDate fields is ignored in topology

The SDAM spec says:

Each time the client checks a server, it MUST replace its description of that server with a new one. This replacement MUST happen even if the new server description compares equal to the previous one, in order to keep client-tracked attributes like last update time and round trip time up to date.

This PR ensures the server description is updated even when equal.

What changed?

Are there any files to ignore?

The work here encompasses changes to the retryable writes logic of the driver, such that the `RetryableWriteError` label becomes the primary means of determining whether an operation will be retried. 4.4+ servers will attach this label server-side, so this change allows us to gracefully remove client-side checking of retryable write errors. NODE-2379

We currently have an expectation that sharded tests are run against a single mongos, but the driver does not correctly discover that a single host passed in is a mongos or not. The only way to do this currently is by specifying the host multiple times. A side effect of this is that we need to deduplicate the seedlist in the legacy Mongos topology type

NODE-2379

A change to use the `RetryableWriteError` label to determine if a write should be retried ignored that non-MongoError's could also be caught. These should be ignored for the purposes of retryability

MongoDB 4.4+ will support removing an extra unnecessary empty exchange during SCRAM handshaking NODE-2301

NODE-2290

NODE-2289

Test operations until now have been built dynamically based on key checking on the arguments object passed in. This approach works but is very brittle. This commit introduces a new way of resolving the test operation through a more formal specification that all operations will eventually be migrated to.

Code for tracking the `maxElectionId` currently assumes that the id is represented in extended JSON. This fix modifies the test runner to parse the extended JSON into BSON, and modified the comparison logic to assume ObjectId NODE-2464

NODE-2460

There is no longer any topology type other than the unified topology, so we default to it in the `connect` operation, and remove all tests that don't work with it.

NODE-2318

This is mostly moving files around to improve maintainability of the codebase. The `core` package is completely removed, as well as all dead code in the codebase. NODE-2485

Now that the unified topology is the only topology, there is no need to test it as a special configuration. Additionally, this removes support for < Node Carbon.

…ibutes fresh

mbroadst

LGTM

mbroadst

Please pull in the spec tests introduced here: mongodb/specifications@5d5946e. They will verify the fix is in.

emadum · 2020-03-10T15:28:25Z

I added the test and verified,

Before:

  1 failing

  1) Server Discovery and Monitoring (spec)
       rs
         Repeated ismaster response must be processed:

      AssertionError: expected {} to contain key 'c:27017'
      + expected - actual

       [
      -  "a:27017"
      -  "b:27017"
      +  "c:27017"
       ]

After:
✓ Repeated ismaster response must be processed

emadum · 2020-03-10T21:22:51Z

Accidentally branched off master, fixed PR here:

#2260

mbroadst added 30 commits January 24, 2020 07:25

doc: remove needless reference to current driver version

d4a51a4

docs: provide basic migration updates for v3.5.xwq

41ee8e0

Merge branch '3.5'

ffe92a9

chore: sync retryable-writes tests

2030a69

test: update retryable-writes test runner to use new format

5f04660

refactor: make errorLabels a private set of MongoError

7bbad9c

chore: resync retryable-writes tests for 4.3 mongos issues

bb4689e

refactor: explicitly return a boolean for errorLabel presence

9b80c24

test: all tests simulating stepdown must now include an error code

6449f04

test: ensure OP_MSG exhaust functional test is run against single

a89d491

test: remove assertion that mongodb-core exists in metadata

79f4c65

feat: use error labels for retryable writes in legacy topologies

fefc165

NODE-2379

fix: only consider MongoError subclasses for retryability

265fe40

A change to use the `RetryableWriteError` label to determine if a write should be retried ignored that non-MongoError's could also be caught. These should be ignored for the purposes of retryability

Merge branch '3.5'

68170da

feat: support shorter SCRAM conversations

6b9ff05

MongoDB 4.4+ will support removing an extra unnecessary empty exchange during SCRAM handshaking NODE-2301

chore: sync CRUD v2 spec tests for 4.4 compat

292fe08

feat: support passing a hint to findOneAndReplace/findOneAndUpdate

faee15b

NODE-2290

feat: support allowDiskUse for find commands

dbc0b37

NODE-2289

chore: sync read-write-concern spec tests

a110ee4

fix: pass options into commandSupportsReadConcern

e855c83

fix: use properly camel cased form of mapReduce for command

c1ed2c1

fix: support write concern provided as string in fromOptions

637f428

test: improve testing of default write concern

11f8792

NODE-2291

refactor: spec test runner should support pre-sessions mongodb

8b8a20c

feat: deprecate oplogReplay for find commands

24155e7

NODE-2423

feat: bump wire protocol version for 4.4

6d3f313

NODE-2150

fix: store name of collection for more informative error messages

979d41e

mbroadst and others added 18 commits February 13, 2020 09:30

doc: document new background option for validateCollection

acdb648

NODE-2269

chore: move sdam spec unit tests from unit/core to unit/sdam

b98a00a

feat: add MONGODB-AWS as a supported auth mechanism

7f3cfba

refactor: don't authenticate monitoring connections

2d607fa

Merge branch '3.5'

c4add7b

fix: don't depend on private node api for Timeout wrapper

e6dc1f4

NODE-2460

docs: fix typo and formatting of bulletted lists

69d10ba

docs: update community links to use MongoDB Community forums

9ad0def

refactor: default to unified topology

ade9a62

There is no longer any topology type other than the unified topology, so we default to it in the `connect` operation, and remove all tests that don't work with it.

feat: remove legacy topology types

6aa2434

NODE-2318

chore: remove shallowClone util helper

79cbe09

refactor: merge core and native error files

8c5197a

refactor: remove the core module

8807508

This is mostly moving files around to improve maintainability of the codebase. The `core` package is completely removed, as well as all dead code in the codebase. NODE-2485

chore: remove evergreen configuration for unified topology

4c852e7

Now that the unified topology is the only topology, there is no need to test it as a special configuration. Additionally, this removes support for < Node Carbon.

doc(examples): remove unused imported module assert

2355997

fix: update server description when equal to keep client-tracked attr…

8a79aad

…ibutes fresh

clean up

e8c4561

emadum requested review from mbroadst and reggi March 9, 2020 20:01

mbroadst approved these changes Mar 9, 2020

View reviewed changes

mbroadst suggested changes Mar 10, 2020

View reviewed changes

add spec test sdam/rs/repeated

bbacab4

emadum requested a review from mbroadst March 10, 2020 15:29

emadum changed the base branch from master to 3.5 March 10, 2020 21:05

Merge branch '3.5' into NODE-2474/update-equal-server-description

223f99b

emadum closed this Mar 10, 2020

emadum deleted the NODE-2474/update-equal-server-description branch April 23, 2020 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update server description when equal #2256

fix: update server description when equal #2256

emadum commented Mar 9, 2020 •

edited

mbroadst left a comment

mbroadst left a comment

emadum commented Mar 10, 2020

emadum commented Mar 10, 2020

fix: update server description when equal #2256

fix: update server description when equal #2256

Conversation

emadum commented Mar 9, 2020 • edited

Description

mbroadst left a comment

Choose a reason for hiding this comment

mbroadst left a comment

Choose a reason for hiding this comment

emadum commented Mar 10, 2020

emadum commented Mar 10, 2020

emadum commented Mar 9, 2020 •

edited