Skip to content

SPEC-1396 Drivers MUST clear connection pools when SDAM monitoring fails due to a network error #665

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 4, 2019
Merged

SPEC-1396 Drivers MUST clear connection pools when SDAM monitoring fails due to a network error #665

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5688252
SPEC-1396 Drivers MUST clear connection pools when SDAM monitoring fa…
p Oct 14, 2019
926d522
Use "network timeout"
p Oct 31, 2019
e8dbd53
Remove spec ticket reference
p Oct 31, 2019
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
6 changes: 3 additions & 3 deletions source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,10 +1308,10 @@ Error handling
Network error during server check
`````````````````````````````````

When a server `check`_ fails due to a network error,
the client SHOULD clear its connection pool for the server:
When a server `check`_ fails due to a network error (including a network timeout),
the client MUST clear its connection pool for the server:
Copy link
Member

Choose a reason for hiding this comment

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

Any ideas of how we can test this? Maybe in the CMAP spec tests we can configure isMaster to fail once with a network error via configureFailPoint, wait for the monitor to run the doomed isMaster, and then assert that the pool has been cleared.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not saying we need tests in this PR. Just spitballing. The current changes LGTM.

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 wrote several tests in mongodb/mongo-ruby-driver#1475 but I would prefer to wait until the unified spec runner work is complete before proposing spec tests that touch both cmap and sdam.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to wait for that? It's a non-goal in the unified spec runner scope to define the format for non-”operations” tests, including CMAP and SDAM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our cmap spec runner does not do any i/o, thus as it is implemented it is unable to set fail points on the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ShaneHarvey It appears that failcommand fail point is ignored for ismaster commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out it does work when ismaster is given in mixed case, https://jira.mongodb.org/browse/SERVER-44414.

if the monitor's socket is bad it is likely that all are.
(See `JAVA-1252 <https://jira.mongodb.org/browse/JAVA-1252>`_.)
(See `JAVA-1252 <https://jira.mongodb.org/browse/JAVA-1252>`_).

Once a server is connected, the client MUST change its type
to Unknown
Expand Down