Skip to content

Commit

Permalink
fix: only configure the latency monitor if a limit is configured (#1527)
Browse files Browse the repository at this point in the history
If the user does not care about the latency monitor, we should not
run an interval sampling the delay as it's expensive and unecessary.

The fix here is to only create a latency monitor in the connection
manager if the user sets a `maxEventLoopDelay` value.
  • Loading branch information
achingbrain authored Dec 21, 2022
1 parent 340e2dd commit 1147550
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
20 changes: 11 additions & 9 deletions src/connection-manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class DefaultConnectionManager extends EventEmitter<ConnectionManagerEven
private readonly opts: ConnectionManagerInit
private readonly connections: Map<string, Connection[]>
private started: boolean
private readonly latencyMonitor: LatencyMonitor
private readonly latencyMonitor?: LatencyMonitor
private readonly startupReconnectTimeout: number
private connectOnStartupController?: TimeoutController
private readonly dialTimeout: number
Expand Down Expand Up @@ -182,10 +182,12 @@ export class DefaultConnectionManager extends EventEmitter<ConnectionManagerEven

this.started = false

this.latencyMonitor = new LatencyMonitor({
latencyCheckIntervalMs: init.pollInterval,
dataEmitIntervalMs: init.pollInterval
})
if (init.maxEventLoopDelay != null && init.maxEventLoopDelay > 0 && init.maxEventLoopDelay !== Infinity) {
this.latencyMonitor = new LatencyMonitor({
latencyCheckIntervalMs: init.pollInterval,
dataEmitIntervalMs: init.pollInterval
})
}

try {
// This emitter gets listened to a lot
Expand Down Expand Up @@ -297,9 +299,9 @@ export class DefaultConnectionManager extends EventEmitter<ConnectionManagerEven
})

// latency monitor
this.latencyMonitor.start()
this.latencyMonitor?.start()
this._onLatencyMeasure = this._onLatencyMeasure.bind(this)
this.latencyMonitor.addEventListener('data', this._onLatencyMeasure)
this.latencyMonitor?.addEventListener('data', this._onLatencyMeasure)

this.started = true
log('started')
Expand Down Expand Up @@ -361,8 +363,8 @@ export class DefaultConnectionManager extends EventEmitter<ConnectionManagerEven
* Stops the Connection Manager
*/
async stop () {
this.latencyMonitor.removeEventListener('data', this._onLatencyMeasure)
this.latencyMonitor.stop()
this.latencyMonitor?.removeEventListener('data', this._onLatencyMeasure)
this.latencyMonitor?.stop()

this.started = false
await this._close()
Expand Down
39 changes: 39 additions & 0 deletions test/connection-manager/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,4 +570,43 @@ describe('libp2p.connections', () => {
expect(denyOutboundUpgradedConnection.getCall(0)).to.have.nested.property('args[0].multihash.digest').that.equalBytes(remoteLibp2p.peerId.multihash.digest)
})
})

describe('latency monitor', () => {
let libp2p: Libp2pNode

afterEach(async () => {
if (libp2p != null) {
await libp2p.stop()
}
})

it('should only start latency monitor if a limit is configured', async () => {
libp2p = await createNode({
config: createBaseOptions({
peerId: peerIds[0],
addresses: {
listen: ['/ip4/127.0.0.1/tcp/0/ws']
}
})
})

expect(libp2p).to.not.have.nested.property('connectionManager.latencyMonitor')

await libp2p.stop()

libp2p = await createNode({
config: createBaseOptions({
peerId: peerIds[0],
addresses: {
listen: ['/ip4/127.0.0.1/tcp/0/ws']
},
connectionManager: {
maxEventLoopDelay: 10000
}
})
})

expect(libp2p).to.have.nested.property('connectionManager.latencyMonitor')
})
})
})

0 comments on commit 1147550

Please sign in to comment.