Skip to content

Commit

Permalink
feat: expand use of error labels for retryable writes
Browse files Browse the repository at this point in the history
The work here encompasses changes to the retryable writes logic of
the driver, such that the `RetryableWriteError` label becomes the
primary means of determining whether an operation will be retried.
4.4+ servers will attach this label server-side, so this change
allows us to gracefully remove client-side checking of retryable
write errors.

NODE-2379
  • Loading branch information
mbroadst committed Feb 2, 2020
1 parent 7bbad9c commit c775a4a
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 19 deletions.
33 changes: 32 additions & 1 deletion lib/core/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ class MongoWriteConcernError extends MongoError {
super(message);
this.name = 'MongoWriteConcernError';

if (result && Array.isArray(result.errorLabels)) {
this[kErrorLabels] = new Set(result.errorLabels);
}

if (result != null) {
this.result = makeWriteConcernResultObject(result);
}
Expand All @@ -189,6 +193,32 @@ const RETRYABLE_ERROR_CODES = new Set([
13436 // NotMasterOrSecondary
]);

const RETRYABLE_WRITE_ERROR_CODES = new Set([
11600, // InterruptedAtShutdown
11602, // InterruptedDueToReplStateChange
10107, // NotMaster
13435, // NotMasterNoSlaveOk
13436, // NotMasterOrSecondary
189, // PrimarySteppedDown
91, // ShutdownInProgress
7, // HostNotFound
6, // HostUnreachable
89, // NetworkTimeout
9001, // SocketException
262 // ExceededTimeLimit
]);

function isRetryableWriteError(error) {
if (error instanceof MongoWriteConcernError) {
return (
RETRYABLE_WRITE_ERROR_CODES.has(error.code) ||
RETRYABLE_WRITE_ERROR_CODES.has(error.result.code)
);
}

return RETRYABLE_WRITE_ERROR_CODES.has(error.code);
}

/**
* Determines whether an error is something the driver should attempt to retry
*
Expand Down Expand Up @@ -284,5 +314,6 @@ module.exports = {
isRetryableError,
isSDAMUnrecoverableError,
isNodeShuttingDownError,
isNetworkTimeoutError
isNetworkTimeoutError,
isRetryableWriteError
};
25 changes: 23 additions & 2 deletions lib/core/sdam/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ const collationNotSupported = require('../utils').collationNotSupported;
const debugOptions = require('../connection/utils').debugOptions;
const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
const isNetworkTimeoutError = require('../error').isNetworkTimeoutError;
const isRetryableWriteError = require('../error').isRetryableWriteError;
const makeStateMachine = require('../utils').makeStateMachine;
const common = require('./common');
const ServerType = common.ServerType;

// Used for filtering out fields for logging
const DEBUG_FIELDS = [
Expand Down Expand Up @@ -404,6 +406,14 @@ Object.defineProperty(Server.prototype, 'clusterTime', {
}
});

function supportsRetryableWrites(server) {
return (
server.description.maxWireVersion >= 6 &&
server.description.logicalSessionTimeoutMinutes &&
server.description.type !== ServerType.Standalone
);
}

function calculateRoundTripTime(oldRtt, duration) {
const alpha = 0.2;
return alpha * duration + (1 - alpha) * oldRtt;
Expand Down Expand Up @@ -449,11 +459,22 @@ function makeOperationHandler(server, options, callback) {
options.session.serverSession.isDirty = true;
}

if (supportsRetryableWrites(server)) {
err.addErrorLabel('RetryableWriteError');
}

if (!isNetworkTimeoutError(err)) {
server.emit('error', err);
}
} else if (isSDAMUnrecoverableError(err)) {
server.emit('error', err);
} else {
// if pre-4.4 server, then add error label if its a retryable write error
if (server.description.maxWireVersion < 9 && isRetryableWriteError(err)) {
err.addErrorLabel('RetryableWriteError');
}

if (isSDAMUnrecoverableError(err)) {
server.emit('error', err);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/core/sdam/topology.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ class Topology extends EventEmitter {

const cb = (err, result) => {
if (!err) return callback(null, result);
if (!isRetryableError(err)) {
if (!err.hasErrorLabel('RetryableWriteError')) {
return callback(err);
}

Expand Down Expand Up @@ -944,7 +944,7 @@ function executeWriteOperation(args, options, callback) {

const handler = (err, result) => {
if (!err) return callback(null, result);
if (!isRetryableError(err)) {
if (!err.hasErrorLabel('RetryableWriteError')) {
err = getMMAPError(err);
return callback(err);
}
Expand Down
6 changes: 5 additions & 1 deletion test/functional/retryable_writes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ function executeScenarioSetup(scenario, test, config, ctx) {
throw err;
}
})
.then(() => (scenario.data ? ctx.collection.insertMany(scenario.data) : {}))
.then(() =>
Array.isArray(scenario.data) && scenario.data.length
? ctx.collection.insertMany(scenario.data)
: {}
)
.then(() => (test.failPoint ? ctx.db.executeDbAdminCommand(test.failPoint) : {}));
}

Expand Down
8 changes: 4 additions & 4 deletions test/spec/retryable-writes/bulkWrite-errorLabels.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@
},
"collection": {
"data": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion test/spec/retryable-writes/bulkWrite-errorLabels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ tests:
errorLabelsOmit: ["RetryableWriteError"]
collection:
data:
- { _id: 1, x: 11 }
- { _id: 2, x: 22 }
- { _id: 3, x: 33 }
2 changes: 1 addition & 1 deletion test/spec/retryable-writes/bulkWrite-serverErrors.json
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
"data": [
{
"_id": 2,
"x": 23
"x": 22
},
{
"_id": 3,
Expand Down
2 changes: 1 addition & 1 deletion test/spec/retryable-writes/bulkWrite-serverErrors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,5 @@ tests:
errorLabelsContain: ["RetryableWriteError"]
collection:
data:
- { _id: 2, x: 23 }
- { _id: 2, x: 22 }
- { _id: 3, x: 33 }
4 changes: 2 additions & 2 deletions test/spec/retryable-writes/findOneAndReplace-errorLabels.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
},
"data": {
"failCommands": [
"findOneAndModify"
"findAndModify"
],
"errorCode": 112,
"errorLabels": [
Expand Down Expand Up @@ -77,7 +77,7 @@
},
"data": {
"failCommands": [
"findOneAndModify"
"findAndModify"
],
"errorCode": 11600,
"errorLabels": []
Expand Down
4 changes: 2 additions & 2 deletions test/spec/retryable-writes/findOneAndReplace-errorLabels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tests:
configureFailPoint: failCommand
mode: { times: 1 }
data:
failCommands: ["findOneAndModify"]
failCommands: ["findAndModify"]
errorCode: 112 # WriteConflict, not a retryable error code
errorLabels: ["RetryableWriteError"] # Override server behavior: send RetryableWriteError label with non-retryable error code
operation:
Expand All @@ -33,7 +33,7 @@ tests:
configureFailPoint: failCommand
mode: { times: 1 }
data:
failCommands: ["findOneAndModify"]
failCommands: ["findAndModify"]
errorCode: 11600 # InterruptedAtShutdown, normally a retryable error code
errorLabels: [] # Override server behavior: do not send RetryableWriteError label with retryable code
operation:
Expand Down
11 changes: 10 additions & 1 deletion test/spec/retryable-writes/insertOne-serverErrors.json
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,16 @@
]
},
"collection": {
"data": []
"data": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
}
]
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion test/spec/retryable-writes/insertOne-serverErrors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -481,4 +481,6 @@ tests:
result:
errorLabelsContain: ["RetryableWriteError"]
collection:
data: []
data:
- { _id: 1, x: 11 }
- { _id: 2, x: 22 }

0 comments on commit c775a4a

Please sign in to comment.