Skip to content

Commit d6f7e14

Browse files
kvwalkerdaprahamian
kvwalker
authored and
daprahamian
committed
fix(sdam): use new connection for node is recovering and not master errors
Fixes NODE-2025 * test(connections): add prose tests for Connections Survive Primary Stepdown Fixes NODE-1750
1 parent 8f570cb commit d6f7e14

File tree

7 files changed

+298
-8
lines changed

7 files changed

+298
-8
lines changed

‎lib/core/error.js

+24-1
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,35 @@ function isRetryableError(error) {
152152
);
153153
}
154154

155+
const SDAM_UNRECOVERABLE_ERROR_CODES = new Set([
156+
91, // ShutdownInProgress
157+
189, // PrimarySteppedDown
158+
10107, // NotMaster
159+
11600, // InterruptedAtShutdown
160+
11602, // InterruptedDueToReplStateChange
161+
13435, // NotMasterNoSlaveOk
162+
13436 // NotMasterOrSecondary
163+
]);
164+
/**
165+
* Determines whether an error is a "node is recovering" error or a "not master" error for SDAM retryability.
166+
* See https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#not-master-and-node-is-recovering
167+
* @param {MongoError|Error} error
168+
*/
169+
function isSDAMUnrecoverableError(error) {
170+
return (
171+
SDAM_UNRECOVERABLE_ERROR_CODES.has(error.code) ||
172+
(error.message &&
173+
(error.message.match(/not master/) || error.message.match(/node is recovering/)))
174+
);
175+
}
176+
155177
module.exports = {
156178
MongoError,
157179
MongoNetworkError,
158180
MongoParseError,
159181
MongoTimeoutError,
160182
MongoWriteConcernError,
161183
mongoErrorContextSymbol,
162-
isRetryableError
184+
isRetryableError,
185+
isSDAMUnrecoverableError
163186
};

‎lib/core/sdam/server.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const MongoParseError = require('../error').MongoParseError;
1414
const MongoNetworkError = require('../error').MongoNetworkError;
1515
const collationNotSupported = require('../utils').collationNotSupported;
1616
const debugOptions = require('../connection/utils').debugOptions;
17+
const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
1718

1819
// Used for filtering out fields for logging
1920
const DEBUG_FIELDS = [
@@ -238,7 +239,13 @@ class Server extends EventEmitter {
238239
return;
239240
}
240241

241-
wireProtocol.command(this, ns, cmd, options, callback);
242+
wireProtocol.command(this, ns, cmd, options, (err, result) => {
243+
if (err && isSDAMUnrecoverableError(err)) {
244+
this.emit('error', err);
245+
}
246+
247+
callback(err, result);
248+
});
242249
}
243250

244251
/**
@@ -343,7 +350,13 @@ function executeWriteOperation(args, options, callback) {
343350
return;
344351
}
345352

346-
return wireProtocol[op](server, ns, ops, options, callback);
353+
return wireProtocol[op](server, ns, ops, options, (err, result) => {
354+
if (err && isSDAMUnrecoverableError(err)) {
355+
server.emit('error', err);
356+
}
357+
358+
callback(err, result);
359+
});
347360
}
348361

349362
function connectEventHandler(server) {

‎lib/core/sdam/topology.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const BSON = require('../connection/utils').retrieveBSON();
1919
const createCompressionInfo = require('../topologies/shared').createCompressionInfo;
2020
const isRetryableError = require('../error').isRetryableError;
2121
const MongoParseError = require('../error').MongoParseError;
22+
const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
2223
const ClientSession = require('../sessions').ClientSession;
2324
const createClientInfo = require('../topologies/shared').createClientInfo;
2425
const MongoError = require('../error').MongoError;
@@ -919,7 +920,7 @@ function serverErrorEventHandler(server, topology) {
919920
new monitoring.ServerClosedEvent(topology.s.id, server.description.address)
920921
);
921922

922-
if (err instanceof MongoParseError) {
923+
if (err instanceof MongoParseError || isSDAMUnrecoverableError(err)) {
923924
resetServerState(server, err, { clearPool: true });
924925
return;
925926
}
@@ -997,10 +998,11 @@ function resetServerState(server, error, options) {
997998
'descriptionReceived',
998999
new ServerDescription(server.description.address, null, { error })
9991000
);
1001+
server.monitor();
10001002
}
10011003

1002-
if (options.clearPool && server.pool) {
1003-
server.pool.reset(() => resetState());
1004+
if (options.clearPool && server.s.pool) {
1005+
server.s.pool.reset(() => resetState());
10041006
return;
10051007
}
10061008

‎lib/operations/command.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ class CommandOperation extends OperationBase {
105105
);
106106
}
107107

108-
const namespace = new MongoDBNamespace(dbName, '$cmd');
108+
const namespace =
109+
this.namespace != null ? this.namespace : new MongoDBNamespace(dbName, '$cmd');
109110

110111
// Execute command
111112
db.s.topology.command(namespace, command, options, (err, result) => {

‎lib/operations/drop.js

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class DropCollectionOperation extends DropOperation {
3232
super(db, options);
3333

3434
this.name = name;
35+
this.namespace = `${db.namespace}.${name}`;
3536
}
3637

3738
_buildCommand() {

‎package-lock.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/functional/connections_stepdown_tests.js

+250
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
'use strict';
2+
3+
const chai = require('chai');
4+
const expect = chai.expect;
5+
6+
describe('Connections survive primary step down', function() {
7+
let client;
8+
9+
beforeEach(function() {
10+
client = this.configuration.newClient(
11+
{ w: 1 },
12+
{ poolSize: 1, retryWrites: false, useUnifiedTopology: true }
13+
);
14+
return client.connect();
15+
});
16+
17+
afterEach(function() {
18+
return client.close();
19+
});
20+
21+
it('getMore iteration', {
22+
metadata: { requires: { mongodb: '>=4.2.0', topology: 'replicaset' } },
23+
test: function() {
24+
const db = client.db('step-down');
25+
let numberOfConnections;
26+
27+
return Promise.resolve()
28+
.then(() =>
29+
db
30+
.admin()
31+
.serverStatus()
32+
.then(result => {
33+
numberOfConnections = result.connections.totalCreated;
34+
})
35+
)
36+
.then(() => db.createCollection('step-down').then(coll => coll.drop({ w: 'majority' })))
37+
.then(() =>
38+
db.createCollection('step-down', { w: 'majority' }).then(collection =>
39+
collection
40+
.insertMany([{ a: 1 }, { a: 2 }, { a: 3 }, { a: 4 }, { a: 5 }], { w: 'majority' })
41+
.then(result => expect(result.insertedCount).to.equal(5))
42+
.then(() => {
43+
const cursor = collection.find({}, { batchSize: 2 });
44+
return cursor
45+
.next()
46+
.then(item => expect(item.a).to.equal(1))
47+
.then(() => cursor.next())
48+
.then(item => expect(item.a).to.equal(2))
49+
.then(() =>
50+
db
51+
.executeDbAdminCommand(
52+
{ replSetStepDown: 5, force: true },
53+
{ readPreference: 'primary' }
54+
)
55+
.then(() => cursor.next())
56+
.then(item => expect(item.a).to.equal(3))
57+
.then(() =>
58+
db
59+
.admin()
60+
.serverStatus()
61+
.then(result =>
62+
expect(result.connections.totalCreated).to.equal(numberOfConnections)
63+
)
64+
)
65+
);
66+
})
67+
)
68+
);
69+
}
70+
});
71+
72+
it('Not Master - Keep Connection Pool', {
73+
metadata: { requires: { mongodb: '>=4.2.0', topology: 'replicaset' } },
74+
test: function() {
75+
const db = client.db('step-down');
76+
let numberOfConnections;
77+
78+
return Promise.resolve()
79+
.then(() =>
80+
db
81+
.admin()
82+
.serverStatus()
83+
.then(result => {
84+
numberOfConnections = result.connections.totalCreated;
85+
})
86+
)
87+
.then(() => db.createCollection('step-down').then(coll => coll.drop({ w: 'majority' })))
88+
.then(() =>
89+
db.createCollection('step-down', { w: 'majority' }).then(collection =>
90+
db
91+
.executeDbAdminCommand({
92+
configureFailPoint: 'failCommand',
93+
mode: { times: 1 },
94+
data: { failCommands: ['insert'], errorCode: 10107 }
95+
})
96+
.then(() =>
97+
collection.insertOne({ test: 1 }).catch(err => expect(err.code).to.equal(10107))
98+
)
99+
.then(() =>
100+
collection.insertOne({ test: 1 }).then(result => {
101+
expect(result.insertedCount).to.equal(1);
102+
})
103+
)
104+
.then(() =>
105+
db.executeDbAdminCommand({ configureFailPoint: 'failCommand', mode: 'off' })
106+
)
107+
.then(() =>
108+
db
109+
.admin()
110+
.serverStatus()
111+
.then(result =>
112+
expect(result.connections.totalCreated).to.equal(numberOfConnections)
113+
)
114+
)
115+
)
116+
);
117+
}
118+
});
119+
120+
it('Not Master - Reset Connection Pool', {
121+
metadata: { requires: { mongodb: '<=4.0.0', topology: 'replicaset' } },
122+
test: function() {
123+
const db = client.db('step-down');
124+
let numberOfConnections;
125+
126+
return Promise.resolve()
127+
.then(() =>
128+
db
129+
.admin()
130+
.serverStatus()
131+
.then(result => {
132+
numberOfConnections = result.connections.totalCreated;
133+
})
134+
)
135+
.then(() => db.createCollection('step-down').then(coll => coll.drop({ w: 'majority' })))
136+
.then(() =>
137+
db.createCollection('step-down', { w: 'majority' }).then(collection =>
138+
db
139+
.executeDbAdminCommand({
140+
configureFailPoint: 'failCommand',
141+
mode: { times: 1 },
142+
data: { failCommands: ['insert'], errorCode: 10107 }
143+
})
144+
.then(() => {
145+
collection
146+
.insertOne({ test: 1 })
147+
.catch(err => expect(err.code).to.equal(10107))
148+
.then(() =>
149+
db.executeDbAdminCommand({ configureFailPoint: 'failCommand', mode: 'off' })
150+
)
151+
.then(() =>
152+
db
153+
.admin()
154+
.serverStatus()
155+
.then(result =>
156+
expect(result.connections.totalCreated).to.equal(numberOfConnections + 1)
157+
)
158+
);
159+
})
160+
)
161+
);
162+
}
163+
});
164+
165+
it('Shutdown in progress - Reset Connection Pool', {
166+
metadata: { requires: { topology: 'replicaset' } },
167+
test: function() {
168+
const db = client.db('step-down');
169+
let numberOfConnections;
170+
171+
return Promise.resolve()
172+
.then(() =>
173+
db
174+
.admin()
175+
.serverStatus()
176+
.then(result => {
177+
numberOfConnections = result.connections.totalCreated;
178+
})
179+
)
180+
.then(() => db.createCollection('step-down').then(coll => coll.drop({ w: 'majority' })))
181+
.then(() =>
182+
db.createCollection('step-down', { w: 'majority' }).then(collection =>
183+
db
184+
.executeDbAdminCommand({
185+
configureFailPoint: 'failCommand',
186+
mode: { times: 1 },
187+
data: { failCommands: ['insert'], errorCode: 91 }
188+
})
189+
.then(() =>
190+
collection.insertOne({ test: 1 }).catch(err => expect(err.code).to.equal(91))
191+
)
192+
.then(() =>
193+
db.executeDbAdminCommand({ configureFailPoint: 'failCommand', mode: 'off' })
194+
)
195+
.then(() =>
196+
db
197+
.admin()
198+
.serverStatus()
199+
.then(result =>
200+
expect(result.connections.totalCreated).to.equal(numberOfConnections + 1)
201+
)
202+
)
203+
)
204+
);
205+
}
206+
});
207+
208+
it('Interrupted at shutdown - Reset Connection Pool', {
209+
metadata: { requires: { topology: 'replicaset' } },
210+
test: function() {
211+
const db = client.db('step-down');
212+
let numberOfConnections;
213+
214+
return Promise.resolve()
215+
.then(() =>
216+
db
217+
.admin()
218+
.serverStatus()
219+
.then(result => {
220+
numberOfConnections = result.connections.totalCreated;
221+
})
222+
)
223+
.then(() => db.createCollection('step-down').then(coll => coll.drop({ w: 'majority' })))
224+
.then(() =>
225+
db.createCollection('step-down', { w: 'majority' }).then(collection =>
226+
db
227+
.executeDbAdminCommand({
228+
configureFailPoint: 'failCommand',
229+
mode: { times: 1 },
230+
data: { failCommands: ['insert'], errorCode: 11600 }
231+
})
232+
.then(() =>
233+
collection.insertOne({ test: 1 }).catch(err => expect(err.code).to.equal(11600))
234+
)
235+
.then(() =>
236+
db.executeDbAdminCommand({ configureFailPoint: 'failCommand', mode: 'off' })
237+
)
238+
.then(() =>
239+
db
240+
.admin()
241+
.serverStatus()
242+
.then(result =>
243+
expect(result.connections.totalCreated).to.equal(numberOfConnections + 1)
244+
)
245+
)
246+
)
247+
);
248+
}
249+
});
250+
});

0 commit comments

Comments
 (0)