CDRIVER-6092 CDRIVER-6262 CDRIVER-6268 implement exponential backoff and jitter in retry loops

[connorsmacd]

Reference

CDRIVER-6092
CDRIVER-6262
CDRIVER-6268

CDRIVER-6092 is the primary task. CDRIVER-6262 and CDRIVER-6268 fix/augment some of the unified tests introduced in this PR.

Summary

This PR implements the exponential retry backoff algorithm defined by DRIVERS-3239.

Refactor existing retry "gotos" into loops

The existing implementation for retryable reads and writes use a simple goto label for iteration. To better align with the client backpressure implementation, I decided traditional loops would be more suitable. Therefore, I chose to first refactor the existing implementation of retryable reads and writes to use loops without making any backpressure-related changes. These changes are found in the following commits:

• Refactor write retry goto into a loop
• Refactor retryable read gotos into loops

Token bucket (mongoc_token_bucket_t)

Client backpressure uses a token bucket as a simple rate limiting mechanism. The token bucket is only used when adaptiveRetries=True is specified as a URI option. Note that the C implementation of the token bucket closely matches the pseudocode found in the spec.

Common exponential retry backoff implementation for both retryable commands and withTransaction

#2198 introduced a very similar exponential retry backoff algorithm for withTransaction. Both algorithms use the same math to compute durations. However, retryable commands use a different growth factor, initial backoff duration, and maximum backoff duration.

For the purposes of code reuse, I introduced mongoc_retry_backoff_generator_t (renaming suggestions are welcome). This component encapsulates retry iteration to model a generator-like interface. Instead of using hardcoded values for the maximum retry attempt, mongoc_retry_backoff_generator_t computes these values programatically. See: Refactor with_transaction to use backoff generator.

Note

I was initially considering putting these changes into a separate PR that only affected withTransaction, but I felt it was hard to contextualize the changes without seeing the retryable command algorithm as well.

First pass implementation

Before attempting to introduce more generic retryable command interface, I first implemented exponential retry backoff in each existing retry loop. The first passes can be found in the following commits:

• Implement retryable write exponential backoff
• Implement retryable read cmd exponential backoff
• Implement cursor exponential retry backoff
• Implement raw command exponential retry backoff

Important

The above implementations were based of a previous version of the spec where the token bucket was used unconditionally. With the most recent version of the spec, the token bucket is only used if the adaptiveRetries=True URI option is passed.

Generic retryable command interface

To increase code reuse and better align with the spec, I refactored the implementation described above to use a generic retry loop component called mongoc_retryable_cmd_t. The function mongoc_retryable_cmd_run models, as closely as possible, the pseudocode found in the spec. The retry loops differ from each other in how the command is executed and how the retry server is selected. To account for this, mongoc_retryable_cmd_t has a v-table-like construct that allows each implementation to customize these steps of the algorithm.

Tip

For reviewers, it may be helpful to compare the commits from the first pass implementation to understand how the various parts of the loops were broken up to use mongoc_retryable_cmd_t.

Other changes

• Changes to ASSERT_CMPDURATION, which was introduced in CDRIVER-6084 CDRIVER-6189 CDRIVER-6206 implement withTransaction exponential retry backoff #2198:
  • Fix typo in ASSERT_CMPDURATION
  • Move ASSERT_CMPDURATION into TestSuite.h
• Drive-by fixes for the unified test runner:
  • Fix use of incorrect bson_error_t
  • Fix lack of error handling in test op

[kevinAlbs]

Sorry for the goofs in the suggested test. Posting drafted feedback since I expect the test failures might be a source of confusion. Will continue reviewing.

[kevinAlbs]

Only substantial remaining comment is suggestion to move token_bucket to the mongoc_topology_t.

[kevinAlbs]

LGTM!