Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ Fields:
field in the server's hello or legacy hello response, in the case that the server reports an address different from
the address the client uses.

- (=) `error`: information about the last error related to this server. Default null.
- (=) `error`: information about the last error related to this server. Default null. MUST contain
or be able to produce a string describing the error.

- `roundTripTime`: the duration of the hello or legacy hello call. Default null.

Expand Down Expand Up @@ -871,7 +872,8 @@ if serverDescription.maxWireVersion >= 17: # MongoDB 6.0+
topologyDescription.maxSetVersion = serverDescription.setVersion
else:
# Stale primary.
# replace serverDescription with a default ServerDescription of type "Unknown"
# replace serverDescription with a default ServerDescription of type "Unknown" and an error
# field describing that that primary was stale
checkIfHasPrimary()
return
else:
Expand All @@ -889,7 +891,9 @@ else:
)
):
# Stale primary.
# replace serverDescription with a default ServerDescription of type "Unknown"
# replace serverDescription with a default ServerDescription of type "Unknown" and an
# error field describing that that primary was stale

checkIfHasPrimary()
return

Expand All @@ -905,7 +909,8 @@ else:
for each server in topologyDescription.servers:
if server.address != serverDescription.address:
if server.type is RSPrimary:
# See note below about invalidating an old primary.
# See note below about invalidating an old primary and an error field describing that
# the primary was stale
replace the server with a default ServerDescription of type "Unknown"

for each address in serverDescription's "hosts", "passives", and "arbiters":
Expand All @@ -921,9 +926,11 @@ checkIfHasPrimary()
```

A note on invalidating the old primary: when a new primary is discovered, the client finds the previous primary (there
should be none or one) and replaces its description with a default ServerDescription of type "Unknown." A multi-threaded
client MUST [request an immediate check](server-monitoring.md#requesting-an-immediate-check) for that server as soon as
possible.
should be none or one) and replaces its description with a default ServerDescription of type "Unknown". Additionally,
the `error` field of the new `ServerDescription` object MUST include a descriptive error explaining that it was
invalidated because the primary was determined to be stale. Drivers MAY additionally specify whether this was due to an
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph is "a note on invalidating the old primary" so I don't think it applies to all the "electionId/setVersion mismatch" case and this sentence should be moved:

Drivers MAY additionally specify whether this was due to an electionId or setVersion mismatch as described in the ServerDescripion.error section.

It should be moved to the paragraph below that starts with "If the server is primary with an obsolete electionId or setVersion,"

electionId or setVersion mismatch. A multi-threaded client MUST
Copy link
Member

Choose a reason for hiding this comment

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

Let's get more specific about what the error should look like so that we can add tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I purposely left this a little more open-ended to give drivers leeway to use the Error/Exception API most native to their language. I can say that more explicitly in the ServerDescription.error section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to cross-reference the ServerDescription.error section

[request an immediate check](server-monitoring.md#requesting-an-immediate-check) for that server as soon as possible.
Copy link
Member

Choose a reason for hiding this comment

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

Should we expand the scope of this ticket to include more info in other cases where we reset a server to "unknown" besides stale primary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think that makes sense to do here. Seems like the only other place we do that is in the handleError function, so I'll take a look at that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there are two places we do this in our handleError logic, but in both cases, the handleError function takes in an error as a parameter. Do we still want to be testing against the errors here?


If the old primary server version is 4.0 or earlier, the client MUST clear its connection pool for the old primary, too:
the connections are all bad because the old primary has closed its sockets. If the old primary server version is 4.2 or
Expand Down
2 changes: 2 additions & 0 deletions source/server-discovery-and-monitoring/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ following keys:

- type: A ServerType name, like "RSSecondary". See [ServerType](../server-discovery-and-monitoring.md#servertype) for
details pertaining to async and multi-threaded drivers.
- error: An optional object with a with a string field containing a string that must be matched
against the message in the actual `ServerDescription.error`
- setName: A string with the expected replica set name, or null.
- setVersion: absent or an integer.
- electionId: absent, null, or an ObjectId.
Expand Down
10 changes: 10 additions & 0 deletions source/tests_that_need_to_change
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/new_primary.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/primary_disconnect_setversion.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/set_version_can_rollback.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/setversion_equal_max_without_electionid.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/setversion_greaterthan_max_without_electionid.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid-pre-6.0.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/setversion_without_electionid.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/stepdown_change_set_name.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid-pre-6.0.yml
https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/tests/rs/use_setversion_without_electionid.yml
Loading