libp2p / go-libp2p

libp2p implementation in Go
MIT License
6.1k stars 1.07k forks source link

fix: Better GC in membacked peerstore #2960

Closed MarcoPolo closed 2 months ago

MarcoPolo commented 2 months ago

Closes #1698

Makes GC ~60x faster (on my synthetic benchmark) under certain circumstances. So by that logic, it should be fine to run this more often.

Adds a min heap list to track the next address to expire. This lets us short circuit gc if we see the next address to expire isn't expired yet.

Also bumps the gc interval from 60 minutes to 1 minute. This should be fine since we don't traverse every address anymore.

This relies on existing tests to assert nothing has broken.

Of course there are many more improvements that could be made. See the linked issue for inspiration. The next lowest hanging fruit is to do something similar to this across peers.

sukunrt commented 2 months ago

I've removed the change that limited the number of signed peer records we could store in the peer store. I removed the limit because:

  1. There's no way to opt out of it.
  2. A rate limiter for new connections from the same IP / subnet is a better solution to the underlying problem.
  3. If we don't limit addresses in the peer store, the benefits are small.

I suggest we introduce the limit in a separate PR, and provide an option to opt out of the limit.

MarcoPolo commented 2 months ago

Changes look good. I appreciate the extra tests. Thank you.