From e7ca7b979badf92d452e9f8e34e24395eb86e751 Mon Sep 17 00:00:00 2001 From: Tanuj Soni Date: Fri, 3 May 2024 19:13:47 +0530 Subject: [PATCH] Refactor NodeList module to improve performance and remove unnecessary node lists code --- src/API.ts | 4 +- src/Data/AccountDataProvider.ts | 4 +- src/Data/Cycles.ts | 2 +- src/Data/Data.ts | 6 +- src/NodeList.ts | 100 +++++++++++--------------------- 5 files changed, 43 insertions(+), 73 deletions(-) diff --git a/src/API.ts b/src/API.ts index b3469729..29199938 100644 --- a/src/API.ts +++ b/src/API.ts @@ -112,7 +112,7 @@ export function registerRoutes(server: FastifyInstance(data) } else { res = Crypto.sign({ - nodeList: NodeList.getList(), + nodeList: [firstNode], joinRequest: P2P.createArchiverJoinRequest(), dataRequestCycle: Data.createDataRequest( P2PTypes.SnapshotTypes.TypeNames.CYCLE, diff --git a/src/Data/AccountDataProvider.ts b/src/Data/AccountDataProvider.ts index 0f28dec0..1214acbc 100644 --- a/src/Data/AccountDataProvider.ts +++ b/src/Data/AccountDataProvider.ts @@ -93,7 +93,7 @@ export const validateAccountDataRequest = ( return { success: false, error: 'Invalid sign object attached' } } const nodePublicKey = sign.owner - if (!Object.prototype.hasOwnProperty.call(NodeList.byPublicKey, nodePublicKey)) { + if (!NodeList.byPublicKey.has(nodePublicKey)) { return { success: false, error: 'This node is not found in the nodelist!' } } if (!servingValidators.has(nodePublicKey) && servingValidators.size >= config.maxValidatorsToServe) { @@ -143,7 +143,7 @@ export const validateAccountDataByListRequest = ( return { success: false, error: 'Invalid sign object attached' } } const nodePublicKey = sign.owner - if (!Object.prototype.hasOwnProperty.call(NodeList.byPublicKey, nodePublicKey)) { + if (!NodeList.byPublicKey.has(nodePublicKey)) { return { success: false, error: 'This node is not found in the nodelist!' } } // TODO: Add max limit check for accountIds list query diff --git a/src/Data/Cycles.ts b/src/Data/Cycles.ts index f4aa04ce..d7403b0e 100644 --- a/src/Data/Cycles.ts +++ b/src/Data/Cycles.ts @@ -82,7 +82,7 @@ export async function processCycles(cycles: P2PTypes.CycleCreatorTypes.CycleData if (currentNetworkMode === 'shutdown') { Logger.mainLogger.debug(Date.now(), `❌ Shutdown Cycle Record received at Cycle #: ${cycle.counter}`) await Utils.sleep(currentCycleDuration) - NodeList.clearNodeListCache() + NodeList.clearNodeLists() await clearDataSenders() setShutdownCycleRecord(cycle) NodeList.toggleFirstNode() diff --git a/src/Data/Data.ts b/src/Data/Data.ts index a8543931..dd5677c1 100644 --- a/src/Data/Data.ts +++ b/src/Data/Data.ts @@ -470,7 +470,7 @@ export async function replaceDataSender(publicKey: NodeList.ConsensusNodeInfo['p } unsubscribeDataSender(publicKey) // eslint-disable-next-line security/detect-object-injection - const node = NodeList.byPublicKey[publicKey] + const node = NodeList.byPublicKey.get(publicKey) if (node) { const nodeIndex = NodeList.activeListByIdSorted.findIndex((node) => node.publicKey === publicKey) if (nodeIndex > -1) { @@ -540,7 +540,7 @@ export function addDataSender(sender: DataSender): void { async function getConsensusRadius(): Promise { // If there is no node, return existing currentConsensusRadius - if (NodeList.getList().length === 0) return currentConsensusRadius + if (NodeList.isEmpty()) return currentConsensusRadius // Define the query function to get the network config from a node const queryFn = async (node): Promise => { @@ -567,7 +567,7 @@ async function getConsensusRadius(): Promise { // Get the list of 10 max random active nodes or the first node if no active nodes are available const nodes = - NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : NodeList.getList().slice(0, 1) + NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()] // Use robustQuery to get the consensusRadius from multiple nodes const tallyItem = await robustQuery( diff --git a/src/NodeList.ts b/src/NodeList.ts index 1e5d120d..82025d43 100644 --- a/src/NodeList.ts +++ b/src/NodeList.ts @@ -64,15 +64,19 @@ export const fromP2PTypesNode = (node: P2PTypes.NodeListTypes.Node): JoinedConse // STATE -const list: ConsensusNodeInfo[] = [] const standbyList: Map = new Map() const syncingList: Map = new Map() const activeList: Map = new Map() + +// Map to get node public key by node Id +const byId: Map = new Map() + +// Map to get node info by public key, stores all nodes +export const byPublicKey: Map = new Map() + +// Array of active nodes sorted by id export let activeListByIdSorted: ConsensusNodeInfo[] = [] -export let byPublicKey: { [publicKey: string]: ConsensusNodeInfo } = {} -let byIpPort: { [ipPort: string]: ConsensusNodeInfo } = {} -export let byId: { [id: string]: ConsensusNodeInfo } = {} -let publicKeyToId: { [publicKey: string]: string } = {} + export let foundFirstNode = false export type SignedNodeList = { @@ -86,17 +90,8 @@ export const realUpdatedTimes: Map = new Map() // METHODS -function getIpPort(node: Node): string { - if (node.ip && node.port) { - return node.ip + ':' + node.port - } else if (node.externalIp && node.externalPort) { - return node.externalIp + ':' + node.externalPort - } - return '' -} - export function isEmpty(): boolean { - return list.length <= 0 + return byPublicKey.size === 0 } type Node = ConsensusNodeInfo | JoinedConsensor @@ -104,20 +99,19 @@ type Node = ConsensusNodeInfo | JoinedConsensor export function addNodes(status: NodeStatus, nodes: Node[]): void { if (nodes.length === 0) return Logger.mainLogger.debug(`Adding ${status} nodes to the list`, nodes.length, nodes) + for (const node of nodes) { - const ipPort = getIpPort(node) // If node not in lists, add it // eslint-disable-next-line security/detect-object-injection - if (byPublicKey[node.publicKey] === undefined && byIpPort[ipPort] === undefined) { - list.push(node) + if (!byPublicKey.has(node.publicKey)) { const key = node.publicKey switch (status) { case NodeStatus.SYNCING: if (standbyList.has(key)) standbyList.delete(key) if (activeList.has(key)) { activeList.delete(key) - activeListByIdSorted = activeListByIdSorted.filter((node) => node.publicKey === key) + activeListByIdSorted = activeListByIdSorted.filter((node) => node.publicKey !== key) } if (syncingList.has(key)) break syncingList.set(node.publicKey, node) @@ -135,18 +129,16 @@ export function addNodes(status: NodeStatus, nodes: Node[]): void { break } /* eslint-disable security/detect-object-injection */ - byPublicKey[node.publicKey] = node - byIpPort[ipPort] = node + byPublicKey.set(node.publicKey, node) /* eslint-enable security/detect-object-injection */ } // If an id is given, update its id if (node.id) { - const entry = byPublicKey[node.publicKey] + const entry = byPublicKey.get(node.publicKey) if (entry) { entry.id = node.id - publicKeyToId[node.publicKey] = node.id - byId[node.id] = node + byId.set(node.id, node.publicKey) } } } @@ -155,13 +147,11 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo if (nodes.length === 0) return Logger.mainLogger.debug('Refreshing nodes', nodes.length, nodes) for (const node of nodes) { - const ipPort = getIpPort(node) // If node not in lists, add it // eslint-disable-next-line security/detect-object-injection - if (byPublicKey[node.publicKey] === undefined && byIpPort[ipPort] === undefined) { + if (!byPublicKey.has(node.publicKey)) { Logger.mainLogger.debug('adding new node during refresh', node.publicKey) - list.push(node) switch (status) { case NodeStatus.SYNCING: syncingList.set(node.publicKey, node) @@ -176,18 +166,16 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo break } - byPublicKey[node.publicKey] = node + byPublicKey.set(node.publicKey, node) // eslint-disable-next-line security/detect-object-injection - byIpPort[ipPort] = node } // If an id is given, update its id if (node.id) { - const entry = byPublicKey[node.publicKey] + const entry = byPublicKey.get(node.publicKey) if (entry) { entry.id = node.id - publicKeyToId[node.publicKey] = node.id - byId[node.id] = node + byId.set(node.id, node.publicKey) } } } @@ -196,38 +184,23 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo export function removeNodes(publicKeys: string[]): void { if (publicKeys.length > 0) Logger.mainLogger.debug('Removing nodes', publicKeys) // Efficiently remove nodes from nodelist - const keysToDelete: Map = new Map() for (const key of publicKeys) { // eslint-disable-next-line security/detect-object-injection - if (byPublicKey[key] === undefined) { + if (!byPublicKey.has(key)) { console.warn(`removeNodes: publicKey ${key} not in nodelist`) continue } - keysToDelete.set(key, true) /* eslint-disable security/detect-object-injection */ - delete byIpPort[getIpPort(byPublicKey[key])] - delete byPublicKey[key] - const id = publicKeyToId[key] - activeListByIdSorted = activeListByIdSorted.filter((node) => node.id !== id) - delete byId[id] - delete publicKeyToId[key] + syncingList.delete(key) + activeList.delete(key) + standbyList.delete(key) + const id = byPublicKey.get(key).id + activeListByIdSorted = activeListByIdSorted.filter((node) => node.id !== byPublicKey.get(key).id) + byId.delete(id) + byPublicKey.delete(key) /* eslint-enable security/detect-object-injection */ } - - if (keysToDelete.size > 0) { - let key: string - for (let i = list.length - 1; i > -1; i--) { - // eslint-disable-next-line security/detect-object-injection - key = list[i].publicKey - if (keysToDelete.has(key)) { - list.splice(i, 1) - if (syncingList.has(key)) syncingList.delete(key) - else if (activeList.has(key)) activeList.delete(key) - else if (standbyList.has(key)) standbyList.delete(key) - } - } - } } export const addStandbyNodes = (nodes: ConsensusNodeInfo[]): void => { @@ -251,7 +224,7 @@ export function setStatus(status: Exclude, publi Logger.mainLogger.debug(`Updating status ${status} for nodes`, publicKeys) for (const key of publicKeys) { // eslint-disable-next-line security/detect-object-injection - const node = byPublicKey[key] + const node = byPublicKey.get(key) if (node === undefined) { console.warn(`setStatus: publicKey ${key} not in nodelist`) continue @@ -281,8 +254,8 @@ export function setStatus(status: Exclude, publi } } -export function getList(): ConsensusNodeInfo[] { - return list +export function getFirstNode(): ConsensusNodeInfo | undefined { + return byPublicKey.values().next().value; } export function getActiveList(id_sorted = true): ConsensusNodeInfo[] { @@ -317,7 +290,7 @@ export const getCachedNodeList = (): SignedNodeList => { for (let index = 0; index < config.N_RANDOM_NODELIST_BUCKETS; index++) { // If we dont have any active nodes, send back the first node in our list - const nodeList = nodeCount < 1 ? getList().slice(0, 1) : getRandomActiveNodes(nodeCount) + const nodeList = nodeCount < 1 ? [getFirstNode()] : getRandomActiveNodes(nodeCount) const sortedNodeList = [...nodeList].sort(byAscendingNodeId) const signedSortedNodeList = Crypto.sign({ nodeList: sortedNodeList, @@ -427,7 +400,7 @@ export function changeNodeListInRestore(): void { } /** Resets/Cleans all the NodeList associated Maps and Array variables/caches */ -export function clearNodeListCache(): void { +export function clearNodeLists(): void { try { activeNodescache.clear() fullNodesCache.clear() @@ -436,11 +409,8 @@ export function clearNodeListCache(): void { realUpdatedTimes.clear() cacheUpdatedTimes.clear() - list.length = 0 - byId = {} - byIpPort = {} - byPublicKey = {} - publicKeyToId = {} + byId.clear() + byPublicKey.clear() activeListByIdSorted = [] } catch (e) { Logger.mainLogger.error('Error thrown in clearNodeListCache', e)