From c5cef290fd3eab076fc9872bce05d10f913e7dc0 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 15 Nov 2021 16:09:08 +0700 Subject: [PATCH 1/3] Delay the first heartbeat using --network.firstHeartBeatDelayMs --- .../cli/src/options/beaconNodeOptions/network.ts | 9 +++++++++ .../cli/test/unit/options/beaconNodeOptions.test.ts | 2 ++ packages/lodestar/src/network/network.ts | 1 + packages/lodestar/src/network/options.ts | 1 + packages/lodestar/src/network/peers/peerManager.ts | 12 +++++++++++- packages/lodestar/test/e2e/network/gossipsub.test.ts | 1 + packages/lodestar/test/e2e/network/network.test.ts | 1 + .../test/e2e/network/peers/peerManager.test.ts | 7 ++++++- packages/lodestar/test/e2e/network/reqresp.test.ts | 1 + 9 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/options/beaconNodeOptions/network.ts b/packages/cli/src/options/beaconNodeOptions/network.ts index 9d9d9d56ace3..3d12fdf27a05 100644 --- a/packages/cli/src/options/beaconNodeOptions/network.ts +++ b/packages/cli/src/options/beaconNodeOptions/network.ts @@ -11,6 +11,7 @@ export interface INetworkArgs { "network.localMultiaddrs": string[]; "network.subscribeAllSubnets": boolean; "network.connectToDiscv5Bootnodes": boolean; + "network.firstHeartBeatDelayMs": number; } export function parseArgs(args: INetworkArgs): IBeaconNodeOptions["network"] { @@ -28,6 +29,7 @@ export function parseArgs(args: INetworkArgs): IBeaconNodeOptions["network"] { localMultiaddrs: args["network.localMultiaddrs"], subscribeAllSubnets: args["network.subscribeAllSubnets"], connectToDiscv5Bootnodes: args["network.connectToDiscv5Bootnodes"], + firstHeartBeatDelayMs: args["network.firstHeartBeatDelayMs"], }; } @@ -95,4 +97,11 @@ export const options: ICliCommandOptions = { defaultDescription: String(defaultOptions.network.connectToDiscv5Bootnodes === true), group: "network", }, + + "network.firstHeartBeatDelayMs": { + type: "number", + description: "Delay the 1st heart beat of Peer Manager after starting Discv5", + defaultDescription: String(defaultOptions.network.firstHeartBeatDelayMs), + group: "network", + }, }; diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 3f72d17c0c69..b81b4ac418d1 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -45,6 +45,7 @@ describe("options / beaconNodeOptions", () => { "network.localMultiaddrs": [], "network.subscribeAllSubnets": true, "network.connectToDiscv5Bootnodes": true, + "network.firstHeartBeatDelayMs": 1000, "sync.isSingleNode": true, "sync.disableProcessAsChainSegment": true, @@ -100,6 +101,7 @@ describe("options / beaconNodeOptions", () => { localMultiaddrs: [], subscribeAllSubnets: true, connectToDiscv5Bootnodes: true, + firstHeartBeatDelayMs: 1000, }, sync: { isSingleNode: true, diff --git a/packages/lodestar/src/network/network.ts b/packages/lodestar/src/network/network.ts index 68befbb8dbcc..78a687c5490c 100644 --- a/packages/lodestar/src/network/network.ts +++ b/packages/lodestar/src/network/network.ts @@ -116,6 +116,7 @@ export class Network implements INetwork { config, peerMetadata, peerRpcScores, + signal, networkEventBus, }, opts diff --git a/packages/lodestar/src/network/options.ts b/packages/lodestar/src/network/options.ts index fec123c91a83..e1291ad392b9 100644 --- a/packages/lodestar/src/network/options.ts +++ b/packages/lodestar/src/network/options.ts @@ -19,6 +19,7 @@ export const defaultDiscv5Options: IDiscv5DiscoveryInputOptions = { export const defaultNetworkOptions: INetworkOptions = { maxPeers: 30, // Allow some room above targetPeers for new inbound peers targetPeers: 25, + firstHeartBeatDelayMs: 1000, localMultiaddrs: ["/ip4/0.0.0.0/tcp/9000"], bootMultiaddrs: [], discv5: defaultDiscv5Options, diff --git a/packages/lodestar/src/network/peers/peerManager.ts b/packages/lodestar/src/network/peers/peerManager.ts index e325aa74087c..91e77ded7930 100644 --- a/packages/lodestar/src/network/peers/peerManager.ts +++ b/packages/lodestar/src/network/peers/peerManager.ts @@ -1,9 +1,10 @@ import LibP2p, {Connection} from "libp2p"; +import {AbortSignal} from "@chainsafe/abort-controller"; import PeerId from "peer-id"; import {IDiscv5DiscoveryInputOptions} from "@chainsafe/discv5"; import {IBeaconConfig} from "@chainsafe/lodestar-config"; import {allForks, altair, phase0} from "@chainsafe/lodestar-types"; -import {ILogger} from "@chainsafe/lodestar-utils"; +import {ILogger, sleep} from "@chainsafe/lodestar-utils"; import {IBeaconChain} from "../../chain"; import {GoodByeReasonCode, GOODBYE_KNOWN_CODES, Libp2pEvent} from "../../constants"; import {IMetrics} from "../../metrics"; @@ -45,6 +46,11 @@ export type PeerManagerOpts = { targetPeers: number; /** The maximum number of peers we allow (exceptions for subnet peers) */ maxPeers: number; + /** + * Delay the 1st heartbeat after starting discv5 + * See https://github.com/ChainSafe/lodestar/issues/3423 + */ + firstHeartBeatDelayMs: number; /** * If null, Don't run discv5 queries, nor connect to cached peers in the peerStore */ @@ -62,6 +68,7 @@ export type PeerManagerModules = { config: IBeaconConfig; peerMetadata: Libp2pPeerMetadataStore; peerRpcScores: IPeerRpcScoreStore; + signal?: AbortSignal; networkEventBus: INetworkEventBus; }; @@ -103,6 +110,7 @@ export class PeerManager { private peerRpcScores: IPeerRpcScoreStore; /** If null, discovery is disabled */ private discovery: PeerDiscovery | null; + private signal: AbortSignal | undefined; private networkEventBus: INetworkEventBus; // A single map of connected peers with all necessary data to handle PINGs, STATUS, and metrics @@ -124,6 +132,7 @@ export class PeerManager { this.config = modules.config; this.peerMetadata = modules.peerMetadata; this.peerRpcScores = modules.peerRpcScores; + this.signal = modules.signal; this.networkEventBus = modules.networkEventBus; this.opts = opts; @@ -142,6 +151,7 @@ export class PeerManager { this.libp2p.connectionManager.on(Libp2pEvent.peerDisconnect, this.onLibp2pPeerDisconnect); this.networkEventBus.on(NetworkEvent.reqRespRequest, this.onRequest); + await sleep(this.opts.firstHeartBeatDelayMs, this.signal); // On start-up will connected to existing peers in libp2p.peerStore, same as autoDial behaviour this.heartbeat(); this.intervals = [ diff --git a/packages/lodestar/test/e2e/network/gossipsub.test.ts b/packages/lodestar/test/e2e/network/gossipsub.test.ts index 7a9988107547..b6da2ef7fe82 100644 --- a/packages/lodestar/test/e2e/network/gossipsub.test.ts +++ b/packages/lodestar/test/e2e/network/gossipsub.test.ts @@ -25,6 +25,7 @@ const opts: INetworkOptions = { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], + firstHeartBeatDelayMs: 1, discv5: null, }; diff --git a/packages/lodestar/test/e2e/network/network.test.ts b/packages/lodestar/test/e2e/network/network.test.ts index 6d807a167163..f4fa6700f5e3 100644 --- a/packages/lodestar/test/e2e/network/network.test.ts +++ b/packages/lodestar/test/e2e/network/network.test.ts @@ -55,6 +55,7 @@ describe("network", function () { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], + firstHeartBeatDelayMs: 1, discv5: { enr, bindAddr: bindAddrUdp, diff --git a/packages/lodestar/test/e2e/network/peers/peerManager.test.ts b/packages/lodestar/test/e2e/network/peers/peerManager.test.ts index efe5e98be2c0..375046d5cc61 100644 --- a/packages/lodestar/test/e2e/network/peers/peerManager.test.ts +++ b/packages/lodestar/test/e2e/network/peers/peerManager.test.ts @@ -85,7 +85,12 @@ describe("network / peers / PeerManager", function () { attnetsService: mockSubnetsService, syncnetsService: mockSubnetsService, }, - {targetPeers: 30, maxPeers: 50, discv5: null} + { + targetPeers: 30, + maxPeers: 50, + discv5: null, + firstHeartBeatDelayMs: 1, + } ); await peerManager.start(); diff --git a/packages/lodestar/test/e2e/network/reqresp.test.ts b/packages/lodestar/test/e2e/network/reqresp.test.ts index 354bbd316933..dab30820d18f 100644 --- a/packages/lodestar/test/e2e/network/reqresp.test.ts +++ b/packages/lodestar/test/e2e/network/reqresp.test.ts @@ -38,6 +38,7 @@ describe("network / ReqResp", function () { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], + firstHeartBeatDelayMs: 1, discv5: null, }; const state = generateState(); From 0da287aa83da546922de73fb99b6a8bb273c4271 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 15 Nov 2021 17:01:42 +0700 Subject: [PATCH 2/3] New network.discv5FirstQueryDelayMs flag --- .../src/options/beaconNodeOptions/network.ts | 8 ++++---- .../unit/options/beaconNodeOptions.test.ts | 4 ++-- packages/lodestar/src/network/network.ts | 1 - packages/lodestar/src/network/options.ts | 2 +- .../lodestar/src/network/peers/discover.ts | 10 ++++++++++ .../lodestar/src/network/peers/peerManager.ts | 19 ++++++++++--------- .../test/e2e/network/gossipsub.test.ts | 2 +- .../lodestar/test/e2e/network/network.test.ts | 2 +- .../e2e/network/peers/peerManager.test.ts | 2 +- .../lodestar/test/e2e/network/reqresp.test.ts | 2 +- 10 files changed, 31 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/options/beaconNodeOptions/network.ts b/packages/cli/src/options/beaconNodeOptions/network.ts index 3d12fdf27a05..dceb7e844670 100644 --- a/packages/cli/src/options/beaconNodeOptions/network.ts +++ b/packages/cli/src/options/beaconNodeOptions/network.ts @@ -11,7 +11,7 @@ export interface INetworkArgs { "network.localMultiaddrs": string[]; "network.subscribeAllSubnets": boolean; "network.connectToDiscv5Bootnodes": boolean; - "network.firstHeartBeatDelayMs": number; + "network.discv5FirstQueryDelayMs": number; } export function parseArgs(args: INetworkArgs): IBeaconNodeOptions["network"] { @@ -29,7 +29,7 @@ export function parseArgs(args: INetworkArgs): IBeaconNodeOptions["network"] { localMultiaddrs: args["network.localMultiaddrs"], subscribeAllSubnets: args["network.subscribeAllSubnets"], connectToDiscv5Bootnodes: args["network.connectToDiscv5Bootnodes"], - firstHeartBeatDelayMs: args["network.firstHeartBeatDelayMs"], + discv5FirstQueryDelayMs: args["network.discv5FirstQueryDelayMs"], }; } @@ -98,10 +98,10 @@ export const options: ICliCommandOptions = { group: "network", }, - "network.firstHeartBeatDelayMs": { + "network.discv5FirstQueryDelayMs": { type: "number", description: "Delay the 1st heart beat of Peer Manager after starting Discv5", - defaultDescription: String(defaultOptions.network.firstHeartBeatDelayMs), + defaultDescription: String(defaultOptions.network.discv5FirstQueryDelayMs), group: "network", }, }; diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index b81b4ac418d1..6fca2b98593f 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -45,7 +45,7 @@ describe("options / beaconNodeOptions", () => { "network.localMultiaddrs": [], "network.subscribeAllSubnets": true, "network.connectToDiscv5Bootnodes": true, - "network.firstHeartBeatDelayMs": 1000, + "network.discv5FirstQueryDelayMs": 1000, "sync.isSingleNode": true, "sync.disableProcessAsChainSegment": true, @@ -101,7 +101,7 @@ describe("options / beaconNodeOptions", () => { localMultiaddrs: [], subscribeAllSubnets: true, connectToDiscv5Bootnodes: true, - firstHeartBeatDelayMs: 1000, + discv5FirstQueryDelayMs: 1000, }, sync: { isSingleNode: true, diff --git a/packages/lodestar/src/network/network.ts b/packages/lodestar/src/network/network.ts index 78a687c5490c..68befbb8dbcc 100644 --- a/packages/lodestar/src/network/network.ts +++ b/packages/lodestar/src/network/network.ts @@ -116,7 +116,6 @@ export class Network implements INetwork { config, peerMetadata, peerRpcScores, - signal, networkEventBus, }, opts diff --git a/packages/lodestar/src/network/options.ts b/packages/lodestar/src/network/options.ts index e1291ad392b9..5be6a53dbff0 100644 --- a/packages/lodestar/src/network/options.ts +++ b/packages/lodestar/src/network/options.ts @@ -19,7 +19,7 @@ export const defaultDiscv5Options: IDiscv5DiscoveryInputOptions = { export const defaultNetworkOptions: INetworkOptions = { maxPeers: 30, // Allow some room above targetPeers for new inbound peers targetPeers: 25, - firstHeartBeatDelayMs: 1000, + discv5FirstQueryDelayMs: 1000, localMultiaddrs: ["/ip4/0.0.0.0/tcp/9000"], bootMultiaddrs: [], discv5: defaultDiscv5Options, diff --git a/packages/lodestar/src/network/peers/discover.ts b/packages/lodestar/src/network/peers/discover.ts index c556bfed56dd..ddc771c2ae85 100644 --- a/packages/lodestar/src/network/peers/discover.ts +++ b/packages/lodestar/src/network/peers/discover.ts @@ -19,6 +19,7 @@ const MAX_CACHED_ENR_AGE_MS = 5 * 60 * 1000; export type PeerDiscoveryOpts = { maxPeers: number; + discv5FirstQueryDelayMs: number; discv5: Omit; }; @@ -86,6 +87,8 @@ export class PeerDiscovery { /** The maximum number of peers we allow (exceptions for subnet peers) */ private maxPeers: number; + private discv5StartMs: number; + private discv5FirstQueryDelayMs: number; constructor(modules: PeerDiscoveryModules, opts: PeerDiscoveryOpts) { const {libp2p, peerRpcScores, metrics, logger, config} = modules; @@ -95,6 +98,8 @@ export class PeerDiscovery { this.logger = logger; this.config = config; this.maxPeers = opts.maxPeers; + this.discv5StartMs = 0; + this.discv5FirstQueryDelayMs = opts.discv5FirstQueryDelayMs; this.discv5 = Discv5.create({ enr: opts.discv5.enr, @@ -119,6 +124,7 @@ export class PeerDiscovery { async start(): Promise { await this.discv5.start(); + this.discv5StartMs = Date.now(); this.discv5.on("discovered", this.onDiscovered); } @@ -196,6 +202,10 @@ export class PeerDiscovery { * Request to find peers. First, looked at cached peers in peerStore */ private async runFindRandomNodeQuery(): Promise { + // Delay the 1st query after starting discv5 + if (Date.now() - this.discv5StartMs <= this.discv5FirstQueryDelayMs) { + return; + } // Run a general discv5 query if one is not already in progress if (this.randomNodeQuery.code === QueryStatusCode.Active) { this.metrics?.discovery.findNodeQueryRequests.inc({action: "ignore"}); diff --git a/packages/lodestar/src/network/peers/peerManager.ts b/packages/lodestar/src/network/peers/peerManager.ts index 91e77ded7930..47c9540c452f 100644 --- a/packages/lodestar/src/network/peers/peerManager.ts +++ b/packages/lodestar/src/network/peers/peerManager.ts @@ -1,10 +1,9 @@ import LibP2p, {Connection} from "libp2p"; -import {AbortSignal} from "@chainsafe/abort-controller"; import PeerId from "peer-id"; import {IDiscv5DiscoveryInputOptions} from "@chainsafe/discv5"; import {IBeaconConfig} from "@chainsafe/lodestar-config"; import {allForks, altair, phase0} from "@chainsafe/lodestar-types"; -import {ILogger, sleep} from "@chainsafe/lodestar-utils"; +import {ILogger} from "@chainsafe/lodestar-utils"; import {IBeaconChain} from "../../chain"; import {GoodByeReasonCode, GOODBYE_KNOWN_CODES, Libp2pEvent} from "../../constants"; import {IMetrics} from "../../metrics"; @@ -47,10 +46,10 @@ export type PeerManagerOpts = { /** The maximum number of peers we allow (exceptions for subnet peers) */ maxPeers: number; /** - * Delay the 1st heartbeat after starting discv5 + * Delay the 1st query after starting discv5 * See https://github.com/ChainSafe/lodestar/issues/3423 */ - firstHeartBeatDelayMs: number; + discv5FirstQueryDelayMs: number; /** * If null, Don't run discv5 queries, nor connect to cached peers in the peerStore */ @@ -68,7 +67,6 @@ export type PeerManagerModules = { config: IBeaconConfig; peerMetadata: Libp2pPeerMetadataStore; peerRpcScores: IPeerRpcScoreStore; - signal?: AbortSignal; networkEventBus: INetworkEventBus; }; @@ -110,7 +108,6 @@ export class PeerManager { private peerRpcScores: IPeerRpcScoreStore; /** If null, discovery is disabled */ private discovery: PeerDiscovery | null; - private signal: AbortSignal | undefined; private networkEventBus: INetworkEventBus; // A single map of connected peers with all necessary data to handle PINGs, STATUS, and metrics @@ -132,12 +129,17 @@ export class PeerManager { this.config = modules.config; this.peerMetadata = modules.peerMetadata; this.peerRpcScores = modules.peerRpcScores; - this.signal = modules.signal; this.networkEventBus = modules.networkEventBus; this.opts = opts; // opts.discv5 === null, discovery is disabled - this.discovery = opts.discv5 && new PeerDiscovery(modules, {maxPeers: opts.maxPeers, discv5: opts.discv5}); + this.discovery = + opts.discv5 && + new PeerDiscovery(modules, { + maxPeers: opts.maxPeers, + discv5FirstQueryDelayMs: opts.discv5FirstQueryDelayMs, + discv5: opts.discv5, + }); const {metrics} = modules; if (metrics) { @@ -151,7 +153,6 @@ export class PeerManager { this.libp2p.connectionManager.on(Libp2pEvent.peerDisconnect, this.onLibp2pPeerDisconnect); this.networkEventBus.on(NetworkEvent.reqRespRequest, this.onRequest); - await sleep(this.opts.firstHeartBeatDelayMs, this.signal); // On start-up will connected to existing peers in libp2p.peerStore, same as autoDial behaviour this.heartbeat(); this.intervals = [ diff --git a/packages/lodestar/test/e2e/network/gossipsub.test.ts b/packages/lodestar/test/e2e/network/gossipsub.test.ts index b6da2ef7fe82..f41c13a0ac8e 100644 --- a/packages/lodestar/test/e2e/network/gossipsub.test.ts +++ b/packages/lodestar/test/e2e/network/gossipsub.test.ts @@ -25,7 +25,7 @@ const opts: INetworkOptions = { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], - firstHeartBeatDelayMs: 1, + discv5FirstQueryDelayMs: 0, discv5: null, }; diff --git a/packages/lodestar/test/e2e/network/network.test.ts b/packages/lodestar/test/e2e/network/network.test.ts index f4fa6700f5e3..080527d26f9d 100644 --- a/packages/lodestar/test/e2e/network/network.test.ts +++ b/packages/lodestar/test/e2e/network/network.test.ts @@ -55,7 +55,7 @@ describe("network", function () { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], - firstHeartBeatDelayMs: 1, + discv5FirstQueryDelayMs: 0, discv5: { enr, bindAddr: bindAddrUdp, diff --git a/packages/lodestar/test/e2e/network/peers/peerManager.test.ts b/packages/lodestar/test/e2e/network/peers/peerManager.test.ts index 375046d5cc61..da7ca8319bf3 100644 --- a/packages/lodestar/test/e2e/network/peers/peerManager.test.ts +++ b/packages/lodestar/test/e2e/network/peers/peerManager.test.ts @@ -89,7 +89,7 @@ describe("network / peers / PeerManager", function () { targetPeers: 30, maxPeers: 50, discv5: null, - firstHeartBeatDelayMs: 1, + discv5FirstQueryDelayMs: 0, } ); await peerManager.start(); diff --git a/packages/lodestar/test/e2e/network/reqresp.test.ts b/packages/lodestar/test/e2e/network/reqresp.test.ts index dab30820d18f..495d859c98b7 100644 --- a/packages/lodestar/test/e2e/network/reqresp.test.ts +++ b/packages/lodestar/test/e2e/network/reqresp.test.ts @@ -38,7 +38,7 @@ describe("network / ReqResp", function () { targetPeers: 1, bootMultiaddrs: [], localMultiaddrs: [], - firstHeartBeatDelayMs: 1, + discv5FirstQueryDelayMs: 0, discv5: null, }; const state = generateState(); From 8dfad5d2cd5ab70b948147446fc086280042c897 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 15 Nov 2021 17:47:29 +0700 Subject: [PATCH 3/3] Address PR comments --- packages/lodestar/src/network/peers/discover.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/lodestar/src/network/peers/discover.ts b/packages/lodestar/src/network/peers/discover.ts index ddc771c2ae85..215096d0c08d 100644 --- a/packages/lodestar/src/network/peers/discover.ts +++ b/packages/lodestar/src/network/peers/discover.ts @@ -203,9 +203,11 @@ export class PeerDiscovery { */ private async runFindRandomNodeQuery(): Promise { // Delay the 1st query after starting discv5 + // See https://github.com/ChainSafe/lodestar/issues/3423 if (Date.now() - this.discv5StartMs <= this.discv5FirstQueryDelayMs) { return; } + // Run a general discv5 query if one is not already in progress if (this.randomNodeQuery.code === QueryStatusCode.Active) { this.metrics?.discovery.findNodeQueryRequests.inc({action: "ignore"});