fix: node discovery extended, empty host info bug fixed, logging enhanced #121
Conversation
yilmazbahadir
force-pushed
the
fix/gh_120-node-discovery-fix
branch
from
5b4cfda
to
0e7f106
Compare
…sk refactored to process chunks
yilmazbahadir
force-pushed
the
fix/gh_120-node-discovery-fix
branch
from
46ac595
to
7ff0257
Compare
yilmazbahadir
requested review from
AnthonyLaw,
OlegMakarenko,
Wayonb,
0x6861746366574,
fboucquez,
gimre-xymcity and
Jaguar0625
yilmazbahadir
force-pushed
the
fix/gh_120-node-discovery-fix
branch
from
fd8225a
to
b3bd5e5
Compare
yilmazbahadir
force-pushed
the
fix/gh_120-node-discovery-fix
branch
from
b3bd5e5
to
5c4b7cc
Compare
src/services/NodeMonitor.ts
Outdated
|
|
||
| for (const nodes of nodeListChunks) { | ||
| logger.info(`Getting node info for chunk of ${nodes.length} nodes`); | ||
| await runTaskInChunks(this.nodeList, this.nodePeersChunkSize, logger, 'fetchAndAddNodeListPeers', async (nodes) => { |
There was a problem hiding this comment.
The this.nodeList will contain all types of nodes.
Should it just use apiNodeList instead of this.nodeList to request the NodePeers?
There was a problem hiding this comment.
Interesting, I specifically created the apiNodeList for this use, must have been regressed during the refactoring, fixed now.
| }); | ||
| logger.info(`[getNodeList] Nodes count from DB: ${nodesFromDb.length}`); | ||
| // adding the nodes from DB to the node list | ||
| this.addNodesToList(nodesFromDb); |
There was a problem hiding this comment.
Seems like dead / disconnected nodes can stay in the list forever, or do we exclude them?
There was a problem hiding this comment.
It's a known issue, explained in the PR description, addressed in issue #122
|
Look good mate. but I believe we need this solution asap #122. |
| @@ -17,6 +17,8 @@ | |||
| "PEER_NODE_PORT": 7900, | |||
| "REQUEST_TIMEOUT": 5000, | |||
| "NUMBER_OF_NODE_REQUEST_CHUNK": 10, | |||
| "NODE_PEERS_REQUEST_CHUNK_SIZE": 50, | |||
| "CHAIN_HEIGHT_REQUEST_CHUNK_SIZE": 10, | |||
| "PREFERRED_NODES": ["*.symboldev.network"], | |||
There was a problem hiding this comment.
As we discussed in the discord, here is the issue created. #125
fixes #120
What was the issue?
As described in issue #120 Statistics-service was not able to discover and list all the nodes in the network. It was using some static node list in the configuration to query the
node/peersand those queries were being fired at the same time therefore most of them was timing out.What's the fix?
node/peersAPI calls in this large list.node/peersendpoint in chunks now, handling the next chunk after the current one is complete, the default chunk size is 50, but it's configurable through the env varNODE_PEERS_REQUEST_CHUNK_SIZE