Implement prose backpressure tests#1946
Implement prose backpressure tests#1946stIncMale wants to merge 2 commits intomongodb:backpressurefrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Client Backpressure prose tests (per the linked spec) for both the synchronous driver and the reactive-streams driver (via the sync adapter).
Changes:
- Introduce
BackpressureProseTestfunctional tests indriver-sync, including failpoint-based retry/backpressure scenarios. - Add a
driver-reactive-streamswrapper test class that reuses the sync prose tests by overriding client creation to useSyncMongoClient.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| driver-sync/src/test/functional/com/mongodb/client/BackpressureProseTest.java | Adds failpoint-driven prose tests for overload retries/backoff behavior. |
| driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/BackpressureProseTest.java | Reuses the sync prose tests for reactive streams by overriding createClient to return SyncMongoClient. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stIncMale
force-pushed
the
backpressureProseTests
branch
from
c1e03b9 to
4c1a2da
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The relevant spec changes: - https://github.com/mongodb/specifications/blob/8a8a7c56429c80b51ec62268dcafc5e5e3c477ef/source/client-backpressure/tests/README.md JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141
stIncMale
force-pushed
the
backpressureProseTests
branch
from
4c1a2da to
3eedce5
Compare
| } | ||
|
|
||
| private static Duration measureFailedInsertDuration(final MongoCollection<Document> collection, final boolean retryBackoff) { | ||
| if (!retryBackoff) { |
There was a problem hiding this comment.
My codereview skill picked this up:
🟡 Timing assertion fragility (Test 1, L83-88) — Still stands. The measureFailedInsertDuration only configures the jitter supplier for the retryBackoff == false case (commented-out line 87:
setTestJitterSupplier(() -> 0)). The retryBackoff == true path has no corresponding jitter override, meaning it will use random jitter when enabled, making the 300ms variance window non-deterministic.Per the spec, the backoff-enabled measurement should also use a controlled jitter value (close to 1). This will need an else branch when the TODO is uncommented.
There was a problem hiding this comment.
Thank you. Done in eb753df.
The crazy part is that I was sure the following step
iv. Configure the random number generator used for jitter to always return a number as close as possible to
1.
does not exist...
This is a 100% valid concern from the standpoint of "implement the damn test the way it is specified". However, given that jitter is guaranteed to be <= 1 when it is random, and that the 300 ms maxTotalBackoffMillis is computed for the upper jitter bound of 1, specifying jitter supplier 1 changes absolutely nothing for the test.
The relevant spec changes:
A concrete commit is specified instead of
masterbecause this is the last commit included in the DRIVERS-3160 Client Backpressure Support epic. Any newer commits are either irrelevant or are included in the DRIVERS-3337 Client Backpressure Improvements epic, which we do not care about for now.AI usage
All modifications were done by the author. AI was used only to review the changes locally (Claude Code), and to review the GitHub PR (Copilot).
JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141