PHPLIB-1719 Exponential backoff and jitter in retry loops

[GromNaN]

Closes PHPLIB-1719

Implements exponential backoff and jitter in retry loops as specified in the Client Backpressure spec.

Changes

• Implements prose tests 1, 3, and 4 from the Client Backpressure spec:
  • Prose 1: Verifies that retry operations use exponential backoff
  • Prose 3: Verifies that overload errors respect a maximum retry count
  • Prose 4: Verifies that overload errors respect the maxAdaptiveRetries option when configured
• Extracts setFixedJitter() helper to Util so it can be shared across spec test suites (was previously duplicated in TransactionsConvenientApi)

Checklist

• Blocked by Require ext-mongodb ^2.3 and test against dev branch #1879 (no longer since ext-mongodb ^2.3 is now required)

[Copilot]

Pull request overview

Updates the library’s retry/backoff behavior and test coverage to align with the Client Backpressure specification, and refactors shared test utilities for controlling jitter deterministically.

Changes:

• Adds Client Backpressure prose tests for exponential backoff behavior and maximum retry count on overload errors.
• Extracts a shared Util::setFixedJitter() helper for spec tests to deterministically control backoff jitter.
• Updates WithTransaction jitter handling logic (used by retry/backoff computations) and gates relevant tests on ext-mongodb >= 2.3.0dev.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/UnifiedSpecTests/Util.php	Adds shared helper to inject a fixed jitter generator into WithTransaction for deterministic timing assertions.
tests/SpecTests/TransactionsConvenientApi/Prose4_RetryBackoffIsEnforcedTest.php	Switches to the shared jitter helper and gates the test on ext-mongodb >= 2.3.0dev.
tests/SpecTests/ClientBackpressure/Prose3_OverloadErrorMaxRetryTest.php	New prose test validating overload errors stop retrying after the maximum retry count.
tests/SpecTests/ClientBackpressure/Prose1_OpRetryExponentialBackoffTest.php	New prose test intended to validate exponential backoff timing under overload errors, using fixed jitter.
src/Operation/WithTransaction.php	Modifies jitter selection logic used for backoff computations in transaction retry loops.

Comments suppressed due to low confidence (1)

src/Operation/WithTransaction.php:216

• The null-check in getJitter() is inverted: when $this->jitterGenerator is null, the code attempts to invoke it as a callable, which will cause a TypeError the first time backoff is computed. This should call the generator only when it is non-null, otherwise fall back to generating a random jitter value.

    private function getJitter(): float
    {
        if ($this->jitterGenerator === null) {
            return ($this->jitterGenerator)();
        }

        // Jitter is a random float from [0, 1]
        // 2 ** 53 is the largest integer that can be represented in a float without losing precision
        return random_int(0, 2 ** 53) / 2 ** 53;

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.75%. Comparing base (17c4013) to head (b547d8d).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff            @@
##               v2.x    #1880   +/-   ##
=========================================
  Coverage     87.75%   87.75%           
  Complexity     3308     3308           
=========================================
  Files           447      447           
  Lines          6607     6607           
=========================================
  Hits           5798     5798           
  Misses          809      809           

Flag	Coverage Δ
6.0-replica_set	86.58% <100.00%> (ø)
6.0-server	82.60% <100.00%> (ø)
6.0-sharded_cluster	86.37% <100.00%> (ø)
8.0-replica_set	87.61% <100.00%> (ø)
8.0-server	83.35% <100.00%> (ø)
8.0-sharded_cluster	87.45% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[alcaeus]

Implemented tests look good, I just added a couple of suggestions for direct links to individual prose tests.
The PR is missing an implementation of prose test 4 (testing maxAdaptiveRetries)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

[Copilot]

The failpoint only adds SystemOverloadedError and RetryableError labels, but WithTransaction::checkForRetryableError() only retries (and calls backoff()) for exceptions with the TransientTransactionError label. As written, execute() will rethrow immediately and jitter/backoff timing assertions won’t be exercising the intended retry path. Consider adjusting the failpoint/error labels (or the operation under test) so it triggers the retry/backoff logic you want to measure.

Suggested change

	'errorLabels' => ['SystemOverloadedError', 'RetryableError'],
	'errorLabels' => ['SystemOverloadedError', 'RetryableError', 'TransientTransactionError'],

[GromNaN]

The PR is missing an implementation of prose test 4 (testing maxAdaptiveRetries)

Good catch, test added.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[GromNaN]

Prose1 test randomly failing; even if it implements the prose test rigorously.

1) MongoDB\Tests\SpecTests\ClientBackpressure\Prose1_OpRetryExponentialBackoffTest::testOperationRetryUsesExponentialBackoff
Failed asserting that 0.378469725 is less than 0.3.

/home/runner/work/mongo-php-library/mongo-php-library/tests/SpecTests/ClientBackpressure/Prose1_OpRetryExponentialBackoffTest.php:42

[GromNaN]

Closing this PR after deeper analysis of Prose test 1.

Core issue

Prose1_OpRetryExponentialBackoffTest is fundamentally incompatible with the current architecture:

1. setFixedJitter targets the wrong layer — it injects a jitter value into WithTransaction.jitterGenerator, but WithTransaction never retries on SystemOverloadedError (only on TransientTransactionError). The injected jitter therefore has no effect on the measured timing.

2. The actual backoff is inside ext-mongodb — ext-mongodb 2.3 handles the overload retry internally (in C) with its own random number generator and the correct constants (BASE_BACKOFF=100ms, 2^n). PHPLIB has no way to control that jitter.

As a result, both measurements (noBackoffTime and withBackoffTime) depend on ext-mongodb's internal random jitter, making the test randomly flaky (typical failure: assertLessThan(0.3, 0.428...)).

Open question

Prose tests 3 and 4 (command count assertions) are not affected by this issue and can be reopened in a separate PR once ext-mongodb 2.3 is stable.

[GromNaN]

The assertion has been updated to reflect the fact that we cannot force a specific jitter value.