Skip to content

CXX-1848 Add sharded cluster test coverage to EVG config #1007

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

Merged
merged 47 commits into from
Aug 15, 2023

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Aug 3, 2023

Description

This PR primarily resolves CXX-1848. Verified by this patch.

This PR syncs both the legacy and unified transactions spec test files, and adds mongos pinning prose tests.

The syncing of spec test files also resolves, or contributes to the partial resolution of, CXX-1547, CXX-2406, CXX-1976, CXX-2221, CXX-2233, CXX-2235, CXX-2393, CXX-2552, and CXX-2690.

Sharded Cluster Behavior Discrepancies

Raw Response Document

There is only one notable change to the behavior of the CXX Driver, which is a bugfix that addresses a discrepancy in behavior of a sharded cluster compared to a single server or replica set when creating index views, as documented in SERVER-78611.

The index_view::create_*() functions return an optional that is empty if the index(es) to be created already exist (note: this is not consistent with the current Drivers spec, which does not return an optional). This does not behave as intended with sharded clusters due to the response fields being wrapped in a "raw" response document instead of being top-level fields.

This PR fixes the check such that the optional is empty as intended if the index views to be created already exist (consistency with single servers and replica sets).

$listLocalSessions and Non-Existent Databases

Sharded clusters appear to behave differently to single servers and replica sets when running $listLocalSessions on a database that does not yet exist, as documented in SERVER-79306. The only change required to account for this discrepancy was updating the aggregation example code to ensure teh database exists prior to running aggregation commands. This may affect more aggregation operations, but they were not exposed in testing.

Legacy Test Runner Changes

The addition of sharded cluster test coverage exposed a multitude of issues with the legacy test runner . These issues include:

  • Use of the obsolete "sharded-replicaset" topology (followup to CXX-2690 Deprecate sharded-replicaset topology type #975).
  • Prose tests missing minimum server requirements (failCommand on mongos pre-4.1, multi-document transactions on pre-4.2).
  • Flaky tailable await cursor tests due to extremely small maxAwaitTimeMS.
  • Failure to ignore configureFailPoint APM events.
  • Failure to check outcome of collection with readPreference=primary + readConcern=local
  • Failure to account for bulkWrite client error exception inconsistencies (see CXX-1679).
  • Flaky test failures due to failure to properly disable regular failPoints on test failure (exception thrown -> disable code not executed).

The only test cases that are skipped is due to CXX-2678 not yet being implemented, which prevents the legacy test runner from being able to specify the readPreference for a runCommand operation.

Note: testing ran into the same issue as CDRIVER-4351. Identified the cause to be the preceeding commitTransaction retry fails on new mongos test in mongos-recovery-token.json failing to properly disable the targetedFailPoint due to failing to trigger the isMaster/hello failPoint as intended (retries were not actually sending a command to the server). Creating a new client object on each attempt resolved the flaky failures. This solution may also be required in the C Driver to address CDRIVER-4351.

Unified Test Runner Changes

Issues that were fixed include:

  • Failure to check outcome of collection with readPreference=primary + readConcern=local.
  • Incorrect specification of hosts in test client connection strings (simplified to hardcoded topology assumptions corresponding to current mongo-orchestration configuration files).

@eramongodb eramongodb self-assigned this Aug 3, 2023
@eramongodb
Copy link
Contributor Author

Latest changes verified by this patch. Merged of upstream branch included redefining sharded cluster tasks to match the new _with_libmongocrypt tasks. Also fixed what appears to have been a mis-merge of #991 and #967 concerning EVG config warnings for the mongocryptd matrix variant (invalid expansion placement).

@eramongodb eramongodb requested a review from kevinAlbs August 9, 2023 21:59
@eramongodb
Copy link
Contributor Author

Latest changes verified by this patch.

@eramongodb eramongodb requested a review from kevinAlbs August 10, 2023 19:23
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The test runner improvements (e.g. capture of APM events on result failure, use of failpoint scope guards) are much appreciated. LGTM!

Copy link
Contributor

@kkloberdanz kkloberdanz left a comment

Choose a reason for hiding this comment

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

Looks good!

@eramongodb eramongodb merged commit 3fe25b8 into mongodb:master Aug 15, 2023
@eramongodb eramongodb deleted the cxx-1848 branch August 15, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants