Improvement: manage stale peer in peer_manager

darshankabariya commented 2 months ago

I've observed something regarding the peer_manager. It appears there is no support for the timely removal of stale peers. The waku_discv5 continually updates the peer_manager with newly introduced peers. However, some nodes may go down or become stale for an extended period. Ideally, we should remove these stale peers from the peer_manager cache to keep it clean and efficient. but that functionality is missing.

There is an attempt is improve the peer manager, it's also resolved the #2414 issue in the peer-exchange side.

https://github.com/waku-org/nwaku/blob/e269dca9cdc56cfd3f29cbabe005f737da5aebe6/waku/node/peer_manager/peer_manager.nim#L395-L397 https://github.com/waku-org/nwaku/blob/e269dca9cdc56cfd3f29cbabe005f737da5aebe6/waku/node/peer_manager/peer_manager.nim#L949-L953

According to Ivan and gabriel, improvements are needed in this area."

darshankabariya commented 2 weeks ago

It appears that modifying the peer manager to handle stale peers isn't necessary, as it largely depends on the health of the discovery process. Hanno also confirmed this in the Nwaku PR. Therefore, I suggest we close this issue.

this issue was created by me while I working on the PX issue, At that time I didn't broader view of peer managers.

However, I'm open to other perspectives.

@SionoiS @gabrielmer @jm-clius

SionoiS commented 2 weeks ago

Maybe not now but I would be in favor of a refactor of nwaku peer manager. To me it seams like the libp2p abstraction is leaking into Waku in this context and creates a big mess (too much "libp2p automagically handle this"). Reading the peer manager code should make it clear how the connection life cycle is handled for each peer and what is the management strategy in place.

gabrielmer commented 4 days ago

Hi @darshankabariya! Could you please elaborate on what's blocking this?

waku-org / nwaku

Improvement: manage stale peer in peer_manager #2905