-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-4098 Bump WIRE_VERSION_MIN from 3 to 6 #926
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done. The test code removal and Evergreen matrix removal seems like a big step towards better maintainability. Thank you for splitting up the diff into logical commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many many small changes, but nothing suspicious. LGTM!
Per CDRIVER-4098:
Therefore, the minimum wire version supported by the C driver should now be wire version 6. Correspondingly, this PR modifies the
WIRE_VERSION_MIN
macro defined in mongoc-client-private.h and raises it from 3 to 6.Bumping
WIRE_VERSION_MIN
The minimum wire version is enforced in the client implementation by _mongoc_topology_description_check_compatible(), which is invoked by mongoc_topology_description_handle_hello(). There are also various feature compatibility checks spread throughout the client, tested as demanded by corresponding operations. These checks have been left unchanged. Followup changes to simplify or remove the client implementation according to the new minimum wire version are not included in this PR.
request
parameter formock_server_replies_to_find()
Numerous tests are written using a pattern similar to the following:
Despite
mock_server_receives_request()
potentially returning aNULL
result (on timeout, such as due to no compatible servers being found), the test code andmock_server_replies_to_find()
assumerequest
is not-null. A precondition check was therefore added to improve the resulting error message when this condition is violated.Minimum wire version compatibility error message
Previously, the C driver reports compatibility failure with the following message:
The hardcoded wire version and server version numbers are replaced with substitution of
WIRE_VERSION_MIN
and a new_mongoc_wire_version_to_server_version()
function to obtain the corresponding server version number:Removal of Evergreen tasks
All tasks designed to be tested against MongoDB server versions earlier than 3.6 (corresponding to wire version < 6) have been removed. 220 tasks were removed in total, bringing the total number of Evergreen tasks for the mongo-c-project down from 1040 to 820.
Minimum wire version check in mock servers
The mock server creation functions were updated to forbid
max_wire_version
arguments with values belowWIRE_VERSION_MIN
, and branches conditioned on wire version < 6 were removed accordingly. Tests that require specifying aminWireVersion
lower thanWIRE_VERSION_MIN
(or amaxWireVersion
greater thanWIRE_VERSION_MAX
) are still free to do so using themock_server_auto_hello()
function, such as intest_wire_version()
.Test code conditioned on wire version < 6
All checks assuming wire version >= 6 were removed for redundancy, and all tests requiring wire version < 6 were removed and remaining test names modified accordingly. The full diff of old tests vs. new tests are as follows:
Wire version integer literals -> wire version feature macros
All instances of the wire version being specified using an integer literal (e.g.
mock_server_with_auto_hello (6)
) were updated to use a corresponding wire version macro instead (e.g.mock_server_with_auto_hello (WIRE_VERSION_OP_MSG)
). Any instances of test code using wire version literals < 6 were updated or removed accordingly.Wire version string literals -> wire version feature macros
Similarly, all instances of the wire version being specified as part of a string literal (e.g.
"'minWireVersion': 6"
) were updated to use a corresponding wire version macro instead (e.g."'minWireVersion': %d"
+WIRE_VERSION_MIN
). Any existing instances of hardcoded'minWireVersion'
were replaced withWIRE_VERSION_MIN
, and any existing instances of'maxWireVersion'
were replaced withWIRE_VERSION_MAX
. Any instances of test code using wire version literals < 6 were updated or removed accordingly.mock_server_receives_bulk_msg()
This function was added primarily to assist with updating the
test_bulk_error_unordered()
test.mock_server_receives_command()
->mock_server_receives_msg()
This constitutes the majority of the changes in this PR. Wire version 6 introduces support for OP_MSG, and the client is implemented to prefer OP_MSG over OP_QUERY if able. Therefore, numerous tests written assuming the mock server receives an OP_QUERY (via
mock_server_receives_command()
ormock_server_receives_query()
) must be updated to expect and handle an OP_MSG (viamock_server_receives_msg()
) instead. Most of the changes followed patterns similar to one of the following:The
mock_server_receives_query()
function cannot yet be removed due to being used for testing support for exhaust cursor fallback behavior.Several minor changes to contents of messages were also required to address new
'$readPreference'
inclusion behavior, new'$writeConcern'
inclusion behavior, and fields requiring explicit'$numberLong'
.As drive-by fixes,
mock_server_auto_endsessions()
andcapture_logs()
were added to several tests to improve both test suite performance and reduce noise in test output.