Skip to content

CDRIVER-5770 Convert retryable write command construction prose tests to spec tests #1800

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 6 commits into from
Dec 3, 2024

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Dec 2, 2024

Summary

Resolves CDRIVER-5770. Adds new retryable writes spec tests. Includes fixes to the unified test runner needed to pass tests.

DRIVERS-943 converts "Command Construction" tests into unified spec tests. The original "Command Construction" tests added in SPEC-969 were not implemented in the C driver.

Use single mongos to insert initialData

Fixes an observed test failure:

test file: snapshot-sessions
Result mismatch:
Expected:  [ { "_id" : { "$numberInt" : "1" }, "x" : { "$numberInt" : "11" } } ]
Actual:    [  ]

This matches spec recommendation:

If the topology is sharded, the test runner SHOULD use a single mongos for handling initialData to avoid possible runtime errors.

Use primary read preference to disable failpoints

Fixes an observed test failure inserting initial data:

error: Failed to send "insert" command with database "retryable-writes-tests": Failed to read 4 bytes: socket error or timeout

The failure appears due to using primaryPreferred read preference to disable failpoints. A previous test marked the primary server Unknown, then disabled the failpoint on a secondary instead of the primary. Failpoints are expected to be configured with primary read preference:

The failPoint operation instructs the test runner to configure a fail point using a "primary" read preference

Additional fixes

  • Remove bson_destroy of bson_t created with tmp_bson.
  • Abort test on failure to disable failpoint, rather than log.
  • Create internal MongoClient after possibly modifying test URI to include multiple mongoses.

@kevinAlbs kevinAlbs marked this pull request as ready for review December 2, 2024 19:41
@kevinAlbs kevinAlbs requested a review from eramongodb December 2, 2024 19:41
Copy link
Contributor

@eramongodb eramongodb left a comment

Choose a reason for hiding this comment

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

Minor question; otherwise, LGTM.

Comment on lines +853 to +854
BSON_APPEND_INT32 (drop_opts, "serverId", 1);
BSON_APPEND_INT32 (create_opts, "serverId", 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious: why a hardcoded serverId vs. pinning via a client session?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I expect either works. Session pinning only occurs on mongos when using a transaction or using a load balancer. Keeping the hard coding for simplicity.

@kevinAlbs kevinAlbs merged commit 8eba3f6 into mongodb:master Dec 3, 2024
45 checks passed
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.

2 participants