Skip to content

Commit

Permalink
fix: only use ServerDescription equality to prevent event emission
Browse files Browse the repository at this point in the history 
SDAM updates should _always_ be processed, an ServerDescription
equality should only be used to prevent superfluous event emission

NODE-2474
  • Loading branch information
@mbroadst
mbroadst committed Mar 25, 2020
1 parent 5fb0264 commit ddf151d
Show file tree
Hide file tree
Showing 7 changed files with 516 additions and 21 deletions.
46 changes: 25 additions & 21 deletions lib/sdam/topology.js
View file
Expand Up @@ -507,10 +507,10 @@ class Topology extends EventEmitter {
}

// If we already know all the information contained in this updated description, then
// we don't need to update anything or emit SDAM events
if (previousServerDescription && previousServerDescription.equals(serverDescription)) {
return;
}
// we don't need to emit SDAM events, but still need to update the description, in order
// to keep client-tracked attributes like last update time and round trip time up to date
const equalDescriptions =
previousServerDescription && previousServerDescription.equals(serverDescription);

// first update the TopologyDescription
this.s.description = this.s.description.update(serverDescription);
Expand All @@ -520,15 +520,17 @@ class Topology extends EventEmitter {
}

// emit monitoring events for this change
this.emit(
'serverDescriptionChanged',
new ServerDescriptionChangedEvent(
this.s.id,
serverDescription.address,
previousServerDescription,
this.s.description.servers.get(serverDescription.address)
)
);
if (!equalDescriptions) {
this.emit(
'serverDescriptionChanged',
new ServerDescriptionChangedEvent(
this.s.id,
serverDescription.address,
previousServerDescription,
this.s.description.servers.get(serverDescription.address)
)
);
}

// update server list from updated descriptions
updateServers(this, serverDescription);
Expand All @@ -538,14 +540,16 @@ class Topology extends EventEmitter {
processWaitQueue(this);
}

this.emit(
'topologyDescriptionChanged',
new TopologyDescriptionChangedEvent(
this.s.id,
previousTopologyDescription,
this.s.description
)
);
if (!equalDescriptions) {
this.emit(
'topologyDescriptionChanged',
new TopologyDescriptionChangedEvent(
this.s.id,
previousTopologyDescription,
this.s.description
)
);
}
}

auth(credentials, callback) {
Expand Down
77 changes: 77 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/primary_mismatched_me_not_removed.json
View file
@@ -0,0 +1,77 @@
{
"description": "Primary mismatched me is not removed",
"uri": "mongodb://localhost:27017,localhost:27018/?replicaSet=rs",
"phases": [
{
"responses": [
[
"localhost:27017",
{
"ok": 1,
"hosts": [
"localhost:27017",
"localhost:27018"
],
"ismaster": true,
"setName": "rs",
"primary": "localhost:27017",
"me": "a:27017",
"minWireVersion": 0,
"maxWireVersion": 7
}
]
],
"outcome": {
"servers": {
"localhost:27017": {
"type": "RSPrimary",
"setName": "rs"
},
"localhost:27018": {
"type": "Unknown",
"setName": null
}
},
"topologyType": "ReplicaSetWithPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
},
{
"responses": [
[
"localhost:27018",
{
"ok": 1,
"hosts": [
"localhost:27017",
"localhost:27018"
],
"ismaster": false,
"secondary": true,
"setName": "rs",
"primary": "localhost:27017",
"me": "localhost:27018",
"minWireVersion": 0,
"maxWireVersion": 7
}
]
],
"outcome": {
"servers": {
"localhost:27017": {
"type": "RSPrimary",
"setName": "rs"
},
"localhost:27018": {
"type": "RSSecondary",
"setName": "rs"
}
},
"topologyType": "ReplicaSetWithPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
}
]
}
73 changes: 73 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/primary_mismatched_me_not_removed.yml
View file
@@ -0,0 +1,73 @@
description: Primary mismatched me is not removed
uri: mongodb://localhost:27017,localhost:27018/?replicaSet=rs

phases: [
{
responses: [
["localhost:27017", {
ok: 1,
hosts: [
"localhost:27017",
"localhost:27018"
],
ismaster: true,
setName: "rs",
primary: "localhost:27017",
# me does not match the primary responder's address, but the server
# is still added because we don't me mismatch check the primary and all
# servers from a primary ismaster are added to the working server set
me: "a:27017",
minWireVersion: 0,
maxWireVersion: 7
}]
],
outcome: {
servers: {
"localhost:27017": {
type: "RSPrimary",
setName: "rs"
},
"localhost:27018": {
type: "Unknown",
setName: null
}
},
topologyType: "ReplicaSetWithPrimary",
logicalSessionTimeoutMinutes: null,
setName: "rs"
}
},
{
responses: [
["localhost:27018", {
ok: 1,
hosts: [
"localhost:27017",
"localhost:27018"
],
ismaster: false,
secondary: true,
setName: "rs",
primary: "localhost:27017",
me: "localhost:27018",
minWireVersion: 0,
maxWireVersion: 7
}]
],
outcome: {
servers: {
"localhost:27017": {
type: "RSPrimary",
setName: "rs"
},
"localhost:27018": {
type: "RSSecondary",
setName: "rs"
}
},
topologyType: "ReplicaSetWithPrimary",
logicalSessionTimeoutMinutes: null,
setName: "rs"
}
}
]
140 changes: 140 additions & 0 deletions test/spec/server-discovery-and-monitoring/rs/repeated.json
View file
@@ -0,0 +1,140 @@
{
"description": "Repeated ismaster response must be processed",
"uri": "mongodb://a,b/?replicaSet=rs",
"phases": [
{
"responses": [
[
"a:27017",
{
"ok": 1,
"ismaster": false,
"secondary": true,
"hidden": true,
"hosts": [
"a:27017",
"c:27017"
],
"setName": "rs",
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"a:27017": {
"type": "RSOther",
"setName": "rs"
},
"b:27017": {
"type": "Unknown"
},
"c:27017": {
"type": "Unknown"
}
},
"topologyType": "ReplicaSetNoPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
},
{
"responses": [
[
"c:27017",
{
"ok": 1,
"ismaster": true,
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"a:27017": {
"type": "RSOther",
"setName": "rs"
},
"b:27017": {
"type": "Unknown"
}
},
"topologyType": "ReplicaSetNoPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
},
{
"responses": [
[
"a:27017",
{
"ok": 1,
"ismaster": false,
"secondary": true,
"hidden": true,
"hosts": [
"a:27017",
"c:27017"
],
"setName": "rs",
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"a:27017": {
"type": "RSOther",
"setName": "rs"
},
"b:27017": {
"type": "Unknown"
},
"c:27017": {
"type": "Unknown"
}
},
"topologyType": "ReplicaSetNoPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
},
{
"responses": [
[
"c:27017",
{
"ok": 1,
"ismaster": true,
"hosts": [
"a:27017",
"c:27017"
],
"setName": "rs",
"minWireVersion": 0,
"maxWireVersion": 6
}
]
],
"outcome": {
"servers": {
"a:27017": {
"type": "RSOther",
"setName": "rs"
},
"c:27017": {
"type": "RSPrimary",
"setName": "rs"
}
},
"topologyType": "ReplicaSetWithPrimary",
"logicalSessionTimeoutMinutes": null,
"setName": "rs"
}
}
]
}

0 comments on commit ddf151d

Please sign in to comment.