refactor(network): Default configuration constants for DiscoveryLayerNode #3000
Conversation
| import { DhtAddress, PeerDescriptor, RingContacts, ServiceID } from '@streamr/dht' | ||
| import { StreamPartID } from '@streamr/utils' | ||
|
|
||
| export const DEFAULT_DISCOVERY_LAYER_KBUCKET_SIZE = 4 |
There was a problem hiding this comment.
https://github.com/streamr-dev/network/blob/main/developer-docs/index.md#naming-counts
| export const DEFAULT_DISCOVERY_LAYER_KBUCKET_SIZE = 4 | |
| export const DEFAULT_DISCOVERY_LAYER_KBUCKET_NODE_COUNT = 4 |
Could be part of a separate PR (in that case, add a bullet point to the "Future improvements" section)
At some point we refactored PeerManager so that it at least partially hides the KBucket terminology, and uses higher level concepts instead (e.g. neighbors, nearby contacts, connections). Does this config option control any of those higher level concepts, or should we keep this name?
There was a problem hiding this comment.
This is used specifically to control the sizes of the k-buckets that the neighbors are placed in. So it's specifically used to tweak the behaviour of kademlia's k-buckets. Simply calling it neighbor-something might be a bit challenging because of that.
|
|
||
| export const DEFAULT_DISCOVERY_LAYER_KBUCKET_SIZE = 4 | ||
| export const DEFAULT_DISCOVERY_LAYER_JOIN_TIMEOUT = 20000 | ||
| export const DEFAULT_DISCOVERY_LAYER_NEIGHBOR_PING_LIMIT = 16 |
There was a problem hiding this comment.
What this limits? E.g. the count of neighbors to ping, or interval of pings, or timeout of ping responses?
There was a problem hiding this comment.
It limits pings if the number of neighbors is above it. Maybe it could be called ie DEFAULT_DISCOVERY_LAYER_NEIGHBOR_COUNT_PING_LIMIT?
## Summary Added constants for default configurations for Layer2 classes. ## Changes - Random node view count is now configured with the nodeViewSize config option ## Future improvements - Do the same for the discovery layer #3000 - Remove remaining magic numbers around ControlLayerNode configuration (Could also clean up on the DHT side). - Could maybe pass some of the defaults from a higher layer (for example the sdk). This will require some thought as some of the configs could be wanted on a per stream partition basis and others for all streams. - Rename all fields with `listSize` to ie `listNodeCount `
Summary
Added constants for default configuration of the DiscoveryLayerNode.
Future Improvements
Add default configuration configs to the ControlLayerNode and DhtNode