Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Commit

Permalink
fix(read-preference): correct server sort for nearest selection
Browse files Browse the repository at this point in the history
The server sorting during `nearest` server selection was incorrectly
using a greater-than sign, instead of following the rules of array
sort comparitors. This fixes the issue for `pickNearest` as well
as `pickNearestMaxStalenessSeconds`, and contributes tests for
both paths.

NODE-1577
  • Loading branch information
mbroadst committed Jul 19, 2018
1 parent 4f2b263 commit dd4eb9a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 9 deletions.
10 changes: 2 additions & 8 deletions lib/topologies/replset_state.js
Expand Up @@ -878,19 +878,14 @@ function pickNearestMaxStalenessSeconds(self, readPreference) {
// Filter by tags
servers = filterByTags(readPreference, servers);

//
// Locate lowest time (picked servers are lowest time + acceptable Latency margin)
// var lowest = servers.length > 0 ? servers[0].lastIsMasterMS : 0;

// Filter by latency
servers = servers.filter(function(s) {
return s.staleness <= maxStalenessMS;
});

// Sort by time
servers.sort(function(a, b) {
// return a.time > b.time;
return a.lastIsMasterMS > b.lastIsMasterMS;
return a.lastIsMasterMS - b.lastIsMasterMS;
});

// No servers, default to primary
Expand Down Expand Up @@ -937,8 +932,7 @@ function pickNearest(self, readPreference) {

// Sort by time
servers.sort(function(a, b) {
// return a.time > b.time;
return a.lastIsMasterMS > b.lastIsMasterMS;
return a.lastIsMasterMS - b.lastIsMasterMS;
});

// Locate lowest time (picked servers are lowest time + acceptable Latency margin)
Expand Down
47 changes: 46 additions & 1 deletion test/tests/unit/replset/read_preference_tests.js
Expand Up @@ -5,8 +5,10 @@ const ReplSet = require('../../../../lib/topologies/replset');
const ReadPreference = require('../../../../lib/topologies/read_preference');
const mock = require('mongodb-mock-server');
const ReplSetFixture = require('../common').ReplSetFixture;
const ReplSetState = require('../../../../lib/topologies/replset_state');
const MongoError = require('../../../..').MongoError;

describe('Secondaries (ReplSet)', function() {
describe('ReadPreference (ReplSet)', function() {
let test;
before(() => (test = new ReplSetFixture()));
afterEach(() => mock.cleanup());
Expand Down Expand Up @@ -48,4 +50,47 @@ describe('Secondaries (ReplSet)', function() {
replSet.connect({ readPreference: new ReadPreference('secondary') });
}
});

it('should correctly sort servers by `lastIsMasterMS` during nearest selection', function() {
const state = new ReplSetState();
const sampleData = [
{ type: 'RSPrimary', lastIsMasterMS: 5 },
{ type: 'RSSecondary', lastIsMasterMS: 4 },
{ type: 'RSSecondary', lastIsMasterMS: 4 },
{ type: 'RSSecondary', lastIsMasterMS: 109 },
{ type: 'RSSecondary', lastIsMasterMS: 110 },
{ type: 'RSSecondary', lastIsMasterMS: 110 },
{ type: 'RSSecondary', lastIsMasterMS: 245 },
{ type: 'RSSecondary', lastIsMasterMS: 221 },
{ type: 'RSSecondary', lastIsMasterMS: 199 },
{ type: 'RSSecondary', lastIsMasterMS: 129 },
{ type: 'RSSecondary', lastIsMasterMS: 131 },
{ type: 'RSSecondary', lastIsMasterMS: 284 },
{ type: 'RSSecondary', lastIsMasterMS: 298 },
{ type: 'RSSecondary', lastIsMasterMS: 289 },
{ type: 'RSSecondary', lastIsMasterMS: 312 }
];

// load mock data into replset state
sampleData.forEach(desc => {
desc.ismaster = { maxWireVersion: 6 }; // for maxStalenessSeconds test
desc.staleness = Math.floor(Math.random() * 100);

if (desc.type === 'RSPrimary') {
state.primary = desc;
} else {
state.secondaries.push(desc);
}
});

// select the nearest server without max staleness seconds
let server = state.pickServer(ReadPreference.nearest);
expect(server).to.not.be.an.instanceOf(MongoError);
expect(server.lastIsMasterMS).to.equal(4);

// select the nearest server with max staleness seconds
server = state.pickServer(new ReadPreference('nearest', { maxStalenessSeconds: 100 }));
expect(server).to.not.be.an.instanceOf(MongoError);
expect(server.lastIsMasterMS).to.equal(4);
});
});

0 comments on commit dd4eb9a

Please sign in to comment.