Migrate to libp2p > 0.35.8 and gossipsub 0.12.2 #3661
Conversation
|
Code Climate has analyzed commit 5040e37 and detected 0 issues on this pull request. View more on Code Climate. |
Codecov Report
@@ Coverage Diff @@
## master #3661 +/- ##
==========================================
- Coverage 36.41% 36.12% -0.30%
==========================================
Files 324 325 +1
Lines 8952 9041 +89
Branches 1403 1419 +16
==========================================
+ Hits 3260 3266 +6
- Misses 5549 5632 +83
Partials 143 143 |
Performance Report✔️ no performance regression detected Full benchmark results
|
|
js-libp2p v0.36.0 just released |
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
from
bd9987c
to
5040e37
Compare
wemeetagain
requested review from
dapplion,
g11tech,
mpetrunic and
wemeetagain
as code owners
|
warning: we'll get the below error when starting the node if we use the old peerstore dir: removing it or pointing to a new peerstore dir should get us through the error |
What the keys that changed encoding? Should there be a migration routine? |
|
I suggest to just change the data key and abandon the old data. No need to prune that |
0.36.x stores all peer data in a single common name space https://github.com/libp2p/js-libp2p/blob/v0.36.0/src/peer-store/store.js#L50 so I don't see we can migrate easily, is peer store data is so valuable to do a migration? |
|
I think we should do something like:
|
How do you differentiate between the old peerstore and the new peerstore? With your idea now Lodestar has to mantain this legacy code migration forever which is annoying |
the other thing is we don't want to maintain old libp2p logic of store/load peer data in lodestar code Can we do this migration step manually? For each release, if there are migration guide we should note it somewhere and include it in the release. I don't see if's critical to keep the existing peer store, I'd go with removing the peerstore folder manually before we deploy the new version. |
|
What's the problem with abandoning the previous data? Can't we do that? |
Right, I was thinking we would use some heuristic to determine if its the old version. Eg: Before startup, fetch a few keys and attempt a pattern match.
Definitely not critical to keep the existing peer store. Its just a matter of what we expect the UX of running and upgrading lodestar to be.
Doing nothing results in the fatal error @tuyennhv posted above. Some intervention is needed to get lodestar to run. |
|
From a UX perspective, breaking this workflow will just require some good communication through the release notes/blog/etc. and it's better to do this now while most of our current users are generally still technical people. I'm ok with either method of a migration guide or @wemeetagain 's script to determine and delete the old peerstore. We should just get moving on this because it's a blocker on some of the other problems we are seeing. |
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
2 times, most recently
from
88c20e8
to
515e687
Compare
|
have tested this branch in contabo-19, it shows good result regarding "Gossip Block Processed Delay" metric, no Unknown Block sync happened while other nodes (contabo-5, same number of validators connected) need helps a lot from UnknownBlock sync Memory is stable (ranges from 2GB - 2.7GB) |
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
2 times, most recently
from
2dc3fb5
to
97e9114
Compare
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
2 times, most recently
from
2268356
to
9be8df8
Compare
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
from
9be8df8
to
5ab4311
Compare
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
from
02b0045
to
85b1c8a
Compare
There was a problem hiding this comment.
I'm really disliking having to convert so many functions to async due to the peerStore. We keep a lot of stuff in memory: all operations pool, state caches, etc. And now we are introducing eager writes and reads for so little data like the peerstore. It doesn't make sense when gossip keeps all its scores in memory.
- score: It's just a number. You can keep thousands and not cause a memory issue. Prune after X time, like 1 hour after disconnection.
- addressBook: Only relevant while the peer is connected, prune after disconnection so bounded by peer count
- metadata: Same as above, only relevant for connected peers
twoeths
force-pushed
the
tuyen/libp2p-0.35
branch
from
85b1c8a
to
44e91e5
Compare
Motivation
We want to migrate to latest versions of libp2p and gossipsub which contains some improvements to be applied to lodestar
Description
Closes #3548
TODO: