Skip to content

DRIVERS-3416 test overload retry when retryReads/Writes=false#1909

Merged
kevinAlbs merged 12 commits intomongodb:masterfrom
kevinAlbs:test-runCommand-retry
Mar 10, 2026
Merged

DRIVERS-3416 test overload retry when retryReads/Writes=false#1909
kevinAlbs merged 12 commits intomongodb:masterfrom
kevinAlbs:test-runCommand-retry

Conversation

@kevinAlbs
Copy link
Contributor

@kevinAlbs kevinAlbs commented Mar 9, 2026

Summary

  • Test overload retry when retryReads/retryWrites is disabled.
  • Test overload retry of aggregate with a write stage.

Background & Motivation

Review of the C driver discovered operations were retried on overload errors when retryReads=false or retryWrites=false, contradicting this spec requirement:

A retry attempt will only be permitted if: [...] The command is a write and retryWrites is enabled or the command is a read and retryReads is enabled.

A test case for aggregate with ($out / $merge) is added as a possible edge case. Unlike other aggregate read operations, Retryable Writes suggests it would be a write operation:

Write commands other than insert, update, delete, or findAndModify will not be initially supported by MongoDB 3.6, although this may change in the future. This includes, but is not limited to, an aggregate command using a write stage (e.g. $out, $merge).

Please complete the following before merging:

  • Is the relevant DRIVERS ticket in the PR title?
  • [ ] Update changelog. Test changes only.
  • Test changes in at least one language driver. (Tested locally in C)
  • Test these changes against all server versions and topologies (including standalone, replica set, and sharded
    clusters). (Not-yet done)

Drive-by fix. Ran: `pre-commit run --all-files mdformat`.
@kevinAlbs kevinAlbs changed the title DRIVERS-3416 test runCommand when retryReads/Writes=false DRIVERS-3416 test overload retry when retryReads/Writes=false Mar 9, 2026
@kevinAlbs
Copy link
Contributor Author

Changes have been extended to all tested operations. And a test for aggregate with a write-stage is added as an edge case.

@connorsmacd here is a rough draft of changes in the C driver to validate the tests: kevinAlbs/mongo-c-driver@eb46d92. This was intended as "just enough" to get tests passing, but that might help as a starting point.

@kevinAlbs kevinAlbs marked this pull request as ready for review March 10, 2026 16:42
@kevinAlbs kevinAlbs requested review from a team as code owners March 10, 2026 16:42
@kevinAlbs kevinAlbs requested review from isabelatkinson and katcharov and removed request for a team, isabelatkinson and katcharov March 10, 2026 16:42
To fix observed error in C driver tests:

```
error: database backpressure-db not found
```

The `*database_name` is used in initialData, creating the database before the test.
@kevinAlbs
Copy link
Contributor Author

fed5e1f intends to address this C driver test failure on sharded clusters:

sending distinct to each mongos
error: database backpressure-db not found

@NoahStapp
Copy link
Contributor

@connorsmacd
Copy link
Contributor

All tests are passing in the C Driver PR as of 4087eb0.

Copy link
Contributor

@connorsmacd connorsmacd left a comment

Choose a reason for hiding this comment

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

LGTM aside from questions about the empty lines in the changelog.

any customers experiencing degraded performance can simply disable `retryableReads`.

## Changelog

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this empty line a placeholder for an eventual changelog update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a drive-by fix to address failing lint tasks on master. That has since been fixed by #1908 so I will git merge master before merging.

retryWrites is not true would be inconsistent with the server and potentially confusing to developers.

## Changelog

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinAlbs kevinAlbs merged commit 5711e4b into mongodb:master Mar 10, 2026
6 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.

3 participants