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

Commit

Permalink
fix(sessions): ensure that we ignore session details from arbiters
Browse files Browse the repository at this point in the history
Arbiters in a ReplSet should not be taken into consideration if
they report a `logicalSessionTimeoutMinutes`.

NODE-1188
  • Loading branch information
mbroadst committed Dec 11, 2017
1 parent 65dcca4 commit de0105c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 34 deletions.
7 changes: 4 additions & 3 deletions lib/topologies/replset_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ ReplSetState.prototype.remove = function(server, options) {
}
};

const isArbiter = ismaster => ismaster.arbiterOnly && ismaster.setName;

ReplSetState.prototype.update = function(server) {
var self = this;
// Get the current ismaster
Expand Down Expand Up @@ -266,7 +268,7 @@ ReplSetState.prototype.update = function(server) {
}

// Update logicalSessionTimeoutMinutes
if (ismaster.logicalSessionTimeoutMinutes !== undefined) {
if (ismaster.logicalSessionTimeoutMinutes !== undefined && !isArbiter(ismaster)) {
if (
self.logicalSessionTimeoutMinutes === undefined ||
ismaster.logicalSessionTimeoutMinutes === null
Expand Down Expand Up @@ -597,8 +599,7 @@ ReplSetState.prototype.update = function(server) {
// Arbiter handling
//
if (
ismaster.arbiterOnly &&
ismaster.setName &&
isArbiter(ismaster) &&
!inList(ismaster, server, this.arbiters) &&
this.setName &&
this.setName === ismaster.setName
Expand Down
81 changes: 50 additions & 31 deletions test/tests/unit/replset/sessions_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var expect = require('chai').expect,
const test = new ReplSetFixture();
describe('Sessions (ReplSet)', function() {
afterEach(() => mock.cleanup());
beforeEach(() => test.setup());
beforeEach(() => test.setup({ ismaster: mock.DEFAULT_ISMASTER_36 }));

it('should track the highest `clusterTime` seen in a replica set', {
metadata: { requires: { topology: 'single' } },
Expand All @@ -31,14 +31,10 @@ describe('Sessions (ReplSet)', function() {
}
);

let serverCount = 0;
replset.on('joined', () => {
serverCount++;
if (serverCount === 3) {
expect(replset.clusterTime).to.eql(futureClusterTime);
replset.destroy();
done();
}
replset.on('all', () => {
expect(replset.clusterTime).to.eql(futureClusterTime);
replset.destroy();
done();
});

replset.on('error', done);
Expand Down Expand Up @@ -67,19 +63,15 @@ describe('Sessions (ReplSet)', function() {
}
);

let serverCount = 0;
replset.on('joined', () => {
serverCount++;
if (serverCount === 3) {
expect(replset.clusterTime).to.eql(futureClusterTime);
const servers = replset.s.replicaSetState.secondaries
.concat(replset.s.replicaSetState.arbiters)
.concat([replset.s.replicaSetState.primary]);
servers.forEach(server => expect(server.clusterTime).to.eql(futureClusterTime));
replset.on('all', () => {
expect(replset.clusterTime).to.eql(futureClusterTime);
const servers = replset.s.replicaSetState.secondaries
.concat(replset.s.replicaSetState.arbiters)
.concat([replset.s.replicaSetState.primary]);
servers.forEach(server => expect(server.clusterTime).to.eql(futureClusterTime));

replset.destroy();
done();
}
replset.destroy();
done();
});

replset.on('error', done);
Expand All @@ -90,6 +82,8 @@ describe('Sessions (ReplSet)', function() {
it('should set `logicalSessionTimeoutMinutes` to `null` if any incoming server is `null`', {
metadata: { requires: { topology: 'single' } },
test: function(done) {
test.firstSecondaryStates[0].logicalSessionTimeoutMinutes = null;

const replset = new ReplSet(
[test.primaryServer.address(), test.firstSecondaryServer.address()],
{
Expand All @@ -102,7 +96,7 @@ describe('Sessions (ReplSet)', function() {
);

replset.on('error', done);
replset.once('connect', () => {
replset.once('all', () => {
expect(replset.logicalSessionTimeoutMinutes).to.equal(null);
replset.destroy();
done();
Expand Down Expand Up @@ -132,20 +126,45 @@ describe('Sessions (ReplSet)', function() {
}
);

let joinCount = 0;
replset.on('joined', () => {
joinCount++;

if (joinCount === 3) {
expect(replset.logicalSessionTimeoutMinutes).to.equal(1);
replset.destroy();
done();
}
replset.on('all', () => {
expect(replset.logicalSessionTimeoutMinutes).to.equal(1);
replset.destroy();
done();
});

replset.on('error', done);
replset.connect();
}
}
);

it('should exclude arbiters when tracking `logicalSessionTimeoutMinutes`', {
metadata: { requires: { topology: 'single' } },
test: function(done) {
test.arbiterServer.setMessageHandler(req => {
const doc = req.document;
if (doc.ismaster) {
req.reply(Object.assign({}, test.arbiterStates[0], { logicalSessionTimeoutMinutes: 2 }));
}
});

const replset = new ReplSet(test.servers.map(s => s.address()), {
setName: 'rs',
connectionTimeout: 3000,
socketTimeout: 0,
haInterval: 100,
size: 1
});

replset.on('joined', type => {
if (type === 'arbiter') {
expect(replset.logicalSessionTimeoutMinutes).to.equal(10);
done();
}
});

replset.on('error', done);
replset.connect();
}
});
});

0 comments on commit de0105c

Please sign in to comment.