diff --git a/doc/LIMITS.md b/doc/LIMITS.md index 8900b06b88..6173b413a7 100644 --- a/doc/LIMITS.md +++ b/doc/LIMITS.md @@ -53,7 +53,7 @@ const node = await createLibp2pNode({ ## Closing connections -When choosing connections to close the connection manager sorts the list of connections by the value derived from the tags given to each peer. The values of all tags are summed and connections with lower valued peers are eligible for closing first. +When choosing connections to close the connection manager sorts the list of connections by the value derived from the tags given to each peer. The values of all tags are summed and connections with lower valued peers are eligible for closing first. If there are tags with equal values, the shortest-lived connection will be closed first. ```js // tag a peer diff --git a/src/connection-manager/index.ts b/src/connection-manager/index.ts index aa4a14a2ba..f158da13e4 100644 --- a/src/connection-manager/index.ts +++ b/src/connection-manager/index.ts @@ -631,6 +631,18 @@ export class DefaultConnectionManager extends EventEmitter connectionBLifespan) { + return -1 + } + return 0 }) diff --git a/test/connection-manager/index.spec.ts b/test/connection-manager/index.spec.ts index 2aadf80125..b5668b2148 100644 --- a/test/connection-manager/index.spec.ts +++ b/test/connection-manager/index.spec.ts @@ -114,6 +114,61 @@ describe('Connection Manager', () => { expect(lowestSpy).to.have.property('callCount', 1) }) + it('should close shortest-lived connection if the tag values are equal', async () => { + const max = 5 + libp2p = await createNode({ + config: createBaseOptions({ + connectionManager: { + maxConnections: max, + minConnections: 2 + } + }), + started: false + }) + + await libp2p.start() + + const connectionManager = libp2p.connectionManager as DefaultConnectionManager + const connectionManagerMaybeDisconnectOneSpy = sinon.spy(connectionManager, '_pruneConnections') + const spies = new Map>>() + + const createConnection = async (value: number, open: number = Date.now(), peerTag: string = 'test-tag') => { + // #TODO: Mock the connection timeline to simulate an older connection + const connection = mockConnection(mockMultiaddrConnection({ ...mockDuplex(), timeline: { open } }, await createEd25519PeerId())) + const spy = sinon.spy(connection, 'close') + + // The lowest tag value will have the longest connection + spies.set(peerTag, spy) + await libp2p.peerStore.tagPeer(connection.remotePeer, peerTag, { + value + }) + + await connectionManager._onConnect(new CustomEvent('connection', { detail: connection })) + } + + // Create one short of enough connections to iniate pruning + for (let i = 1; i < max; i++) { + const value = i * 10 + await createConnection(value) + } + + const value = 0 * 10 + // Add a connection with the lowest tag value BUT the longest lived connection + await createConnection(value, 18000, 'longest') + // Add one more connection with the lowest tag value BUT the shortest-lived connection + await createConnection(value, Date.now(), 'shortest') + + // get the lowest tagged value, but this would be also the longest lived connection + const longestLivedWithLowestTagSpy = spies.get('longest') + + // Get lowest tagged connection but with a shorter-lived connection + const shortestLivedWithLowestTagSpy = spies.get('shortest') + + expect(connectionManagerMaybeDisconnectOneSpy.callCount).to.equal(1) + expect(longestLivedWithLowestTagSpy).to.have.property('callCount', 0) + expect(shortestLivedWithLowestTagSpy).to.have.property('callCount', 1) + }) + it('should close connection when the maximum has been reached even without tags', async () => { const max = 5 libp2p = await createNode({