Skip to content

Commit 5396516

Browse files
Add tests for retry loop and backoff.
1 parent 83c9b33 commit 5396516

15 files changed

+10279
-10
lines changed

src/cmap/connect.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ export interface HandshakeDocument extends Document {
209209
compression: string[];
210210
saslSupportedMechs?: string;
211211
loadBalanced?: boolean;
212+
backpressure: true;
212213
}
213214

214215
/**
@@ -226,6 +227,7 @@ export async function prepareHandshakeDocument(
226227

227228
const handshakeDoc: HandshakeDocument = {
228229
[serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1,
230+
backpressure: true,
229231
helloOk: true,
230232
client: clientMetadata,
231233
compression: compressors

src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export {
8787
MongoWriteConcernError,
8888
WriteConcernErrorResult
8989
} from './error';
90+
export { TokenBucket } from './token_bucket';
9091
export {
9192
AbstractCursor,
9293
// Actual driver classes exported

src/operations/execute_operation.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
} from '../utils';
3939
import { AggregateOperation } from './aggregate';
4040
import { AbstractOperation, Aspect } from './operation';
41+
import { RunCommandOperation } from './run_command';
4142

4243
const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation;
4344
const MMAPv1_RETRY_WRITES_ERROR_MESSAGE =
@@ -265,22 +266,22 @@ async function executeOperationWithRetries<
265266
if (previousOperationError.hasErrorLabel(MongoErrorLabel.SystemOverloadedError)) {
266267
systemOverloadRetryAttempt += 1;
267268

269+
// if retryable writes or reads are not configured, throw.
270+
const isOperationConfiguredForRetry =
271+
(hasReadAspect && topology.s.options.retryReads) ||
272+
(hasWriteAspect && topology.s.options.retryWrites);
273+
const isRunCommand = operation instanceof RunCommandOperation;
274+
268275
if (
269276
// if the SystemOverloadError is not retryable, throw.
270277
!previousOperationError.hasErrorLabel(MongoErrorLabel.RetryableError) ||
271-
!(
272-
// if retryable writes or reads are not configured, throw.
273-
(
274-
(hasReadAspect && topology.s.options.retryReads) ||
275-
(hasWriteAspect && topology.s.options.retryWrites)
276-
)
277-
)
278+
!(isOperationConfiguredForRetry || isRunCommand)
278279
) {
279280
throw previousOperationError;
280281
}
281282

282283
// if we have exhausted overload retry attempts, throw.
283-
if (systemOverloadRetryAttempt >= maxSystemOverloadRetryAttempts) {
284+
if (systemOverloadRetryAttempt > maxSystemOverloadRetryAttempts) {
284285
throw previousOperationError;
285286
}
286287

@@ -361,7 +362,10 @@ async function executeOperationWithRetries<
361362

362363
try {
363364
// If attempt > 0 and we are command batching we need to reset the batch.
364-
if (nonOverloadRetryAttempt > 0 && operation.hasAspect(Aspect.COMMAND_BATCHING)) {
365+
if (
366+
(nonOverloadRetryAttempt > 0 || systemOverloadRetryAttempt > 0) &&
367+
operation.hasAspect(Aspect.COMMAND_BATCHING)
368+
) {
365369
operation.resetBatch();
366370
}
367371

src/token_bucket.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
/**
2+
* @internal
3+
*/
14
export class TokenBucket {
25
private budget: number;
36
constructor(allowance: number) {

sync.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
3+
cp ~/dev/specifications/source/client-backpressure/tests/* ~/dev/node-mongodb-native/test/spec/client-backpressure
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { expect } from 'chai';
2+
import * as sinon from 'sinon';
3+
4+
import { type Collection, type MongoClient, MongoServerError } from '../../../src';
5+
import { clearFailPoint, configureFailPoint, measureDuration } from '../../tools/utils';
6+
7+
describe('Client Backpressure (Prose)', function () {
8+
let client: MongoClient;
9+
let collection: Collection;
10+
11+
beforeEach(async function () {
12+
client = this.configuration.newClient();
13+
await client.connect();
14+
15+
collection = client.db('foo').collection('bar');
16+
});
17+
18+
afterEach(async function () {
19+
await client.close();
20+
await clearFailPoint(this.configuration);
21+
});
22+
23+
it(
24+
'Test 1: Operation Retry Uses Exponential Backoff',
25+
{
26+
requires: {
27+
mongodb: '4.4'
28+
}
29+
},
30+
async function () {
31+
await configureFailPoint(this.configuration, {
32+
configureFailPoint: 'failCommand',
33+
mode: 'alwaysOn',
34+
data: {
35+
failCommands: ['insert'],
36+
errorCode: 2,
37+
errorLabels: ['SystemOverloadedError', 'RetryableError']
38+
}
39+
});
40+
41+
const stub = sinon.stub(Math, 'random');
42+
43+
stub.returns(0);
44+
45+
const { duration: durationNoBackoff } = await measureDuration(async () => {
46+
const error = await collection.insertOne({ a: 1 }).catch(e => e);
47+
expect(error).to.be.instanceof(MongoServerError);
48+
});
49+
50+
stub.returns(1);
51+
52+
const { duration: durationBackoff } = await measureDuration(async () => {
53+
const error = await collection.insertOne({ a: 1 }).catch(e => e);
54+
expect(error).to.be.instanceof(MongoServerError);
55+
});
56+
57+
expect(durationBackoff - durationNoBackoff).to.be.within(3100 - 1000, 3100 + 1000);
58+
}
59+
);
60+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { loadSpecTests } from '../../spec';
2+
import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
3+
import { type Test } from '../../tools/unified-spec-runner/schema';
4+
5+
const skippedTests = {
6+
'collection.dropIndexes retries at most maxAttempts=5 times':
7+
'TODO(NODE-6517): dropIndexes squashes all errors other than ns not found'
8+
};
9+
10+
function shouldSkip({ description }: Test) {
11+
return skippedTests[description] ?? false;
12+
}
13+
14+
describe('Client Backpressure (spec)', function () {
15+
runUnifiedSuite(loadSpecTests('client-backpressure'), shouldSkip);
16+
});

test/integration/mongodb-handshake/mongodb-handshake.prose.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { expect } from 'chai';
22
import * as sinon from 'sinon';
33

4-
import { type ClientMetadata, type DriverInfo, Int32, MongoClient } from '../../../src';
4+
import {
5+
type ClientMetadata,
6+
type Document,
7+
type DriverInfo,
8+
type HandshakeDocument,
9+
Int32,
10+
MongoClient
11+
} from '../../../src';
512
import { Connection } from '../../../src/cmap/connection';
613
import { getFAASEnv, isDriverInfoEqual } from '../../../src/cmap/handshake/client_metadata';
714
import { LEGACY_HELLO_COMMAND } from '../../../src/constants';
@@ -939,3 +946,38 @@ describe('Client Metadata Update Prose Tests', function () {
939946
}
940947
});
941948
});
949+
950+
// TODO: add prose test descriptions here to align the test with the spec.
951+
describe('Backpressure Metadata', function () {
952+
let client: MongoClient;
953+
let spy: sinon.SinonSpy<Parameters<typeof Connection.prototype.command>>;
954+
955+
beforeEach(async function () {
956+
client = this.configuration.newClient();
957+
spy = sinon.spy(Connection.prototype, 'command');
958+
await client.connect();
959+
960+
// run an operation to force a connection establishment,
961+
// if we're testing noauth load balanced mode.
962+
await client.db('foo').collection('bar').insertOne({ name: 'bumpy' });
963+
});
964+
965+
afterEach(async function () {
966+
sinon.restore();
967+
await client?.close();
968+
});
969+
970+
it('includes backpressure in the handshake document', function () {
971+
const isHello = (cmd: Document): cmd is HandshakeDocument =>
972+
`hello` in cmd || LEGACY_HELLO_COMMAND in cmd;
973+
974+
const hellos = spy.args.map(([_ns, command, _options]) => command).filter(isHello);
975+
976+
expect(hellos.length).to.be.greaterThan(0);
977+
978+
expect(
979+
hellos.every(hello => hello.backpressure === true),
980+
`some handshake documents did not specify backpressure: true`
981+
);
982+
});
983+
});
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Client Backpressure Tests
2+
3+
______________________________________________________________________
4+
5+
## Introduction
6+
7+
The YAML and JSON files in this directory are platform-independent tests meant to exercise a driver's implementation of
8+
retryable reads. These tests utilize the [Unified Test Format](../../unified-test-format/unified-test-format.md).
9+
10+
Several prose tests, which are not easily expressed in YAML, are also presented in this file. Those tests will need to
11+
be manually implemented by each driver.
12+
13+
### Prose Tests
14+
15+
#### Test 1: Operation Retry Uses Exponential Backoff
16+
17+
Drivers should test that retries do not occur immediately when a SystemOverloadedError is encountered.
18+
19+
1. let `client` be a `MongoClient`
20+
2. let `collection` be a collection
21+
3. Now, run transactions without backoff:
22+
1. Configure the random number generator used for jitter to always return `0` -- this effectively disables backoff.
23+
24+
2. Configure the following failPoint:
25+
26+
```javascript
27+
{
28+
configureFailPoint: 'failCommand',
29+
mode: 'alwaysOn',
30+
data: {
31+
failCommands: ['insert'],
32+
errorCode: 2,
33+
errorLabels: ['SystemOverloadedError', 'RetryableError']
34+
}
35+
}
36+
```
37+
38+
3. Execute the following command. Expect that the command errors. Measure the duration of the command execution.
39+
40+
```javascript
41+
const start = performance.now();
42+
expect(
43+
await coll.insertOne({ a: 1 }).catch(e => e)
44+
).to.be.an.instanceof(MongoServerError);
45+
const end = performance.now();
46+
```
47+
48+
4. Configure the random number generator used for jitter to always return `1`.
49+
50+
5. Execute step 3 again.
51+
52+
6. Compare the two time between the two runs.
53+
```python
54+
assertTrue(with_backoff_time - no_backoff_time >= 2.1)
55+
```
56+
The sum of 5 backoffs is 3.1 seconds. There is a 1-second window to account for potential variance between the two
57+
runs.
58+
59+
## Changelog
60+
61+
- 2025-XX-XX: Initial version.

0 commit comments

Comments
 (0)