Skip to content

Comments

Peer manager heartbeat#158

Merged
jking-aus merged 3 commits intosigp:unstablefrom
dknopik:improve-peer-store
Mar 6, 2025
Merged

Peer manager heartbeat#158
jking-aus merged 3 commits intosigp:unstablefrom
dknopik:improve-peer-store

Conversation

@dknopik
Copy link
Member

@dknopik dknopik commented Feb 24, 2025

Implement basic heartbeat for peer manager.

Currently, we only check if we need more peers for some subnets, and try to dial if we find some good peers in the peer store. If we can not dial enough peers, we try to discover more.

@dknopik dknopik added ready-for-review This PR is ready to be reviewed network labels Feb 24, 2025
@dknopik dknopik requested a review from jking-aus February 24, 2025 16:03
@dknopik
Copy link
Member Author

dknopik commented Feb 26, 2025

NOTE: converted to draft as I found a bug

@dknopik dknopik marked this pull request as draft February 26, 2025 16:53
@dknopik dknopik removed the ready-for-review This PR is ready to be reviewed label Feb 26, 2025
@dknopik dknopik added the ready-for-review This PR is ready to be reviewed label Mar 4, 2025
@dknopik dknopik marked this pull request as ready for review March 4, 2025 15:54
Copy link
Member

@jking-aus jking-aus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - just note comment to consider pruning unreachable peers

(!actions.discover.is_empty() || !actions.dial.is_empty()).then_some(actions)
}

fn candidate_peers(&self) -> Vec<(&PeerId, &PeerRecord<Enr>)> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we ever prune unreachable ENRs? appreciate a restart would fix this but might be worth removing addresses that fail to connect from peer manager? Apologies if this behaviour is captured in libp2p somewhere and I'm unaware

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already metioned this to João. libp2p should handle it. not sure if it made it in in the currently merged version though

@jking-aus jking-aus merged commit ea73bd0 into sigp:unstable Mar 6, 2025
14 of 20 checks passed
@dknopik dknopik deleted the improve-peer-store branch June 20, 2025 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

network ready-for-review This PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants