Skip to content

chore: deps: update go-libp2p to v0.21#8970

Merged
magik6k merged 15 commits intofilecoin-project:masterfrom
MarcoPolo:update-go-libp2p-v0.21
Aug 19, 2022
Merged

chore: deps: update go-libp2p to v0.21#8970
magik6k merged 15 commits intofilecoin-project:masterfrom
MarcoPolo:update-go-libp2p-v0.21

Conversation

@MarcoPolo
Copy link
Copy Markdown
Contributor

@MarcoPolo MarcoPolo commented Jul 5, 2022 •
edited
Loading

Related Issues

Proposed Changes

Draft PR to help update lotus to use the latest release of go-libp2p.

Also adds observability to the resource manager via prometheus so operators can debug blocked resources better. See https://ipfs.marcopolo.io/d/MgmGIjjnk/resource-manager?orgId=1 for a live example.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • All commits have a clear commit message.
  • The PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
    • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
  • This PR has tests for new functionality or change in behaviour
  • If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
  • CI is green
  • We should merge Update go libp2p v0.21 go-fil-markets#744 first and we can update the go-fil-markets dep here as well.
  • Merge and release Chore: update to go-libp2p 0.21 libp2p/go-libp2p-kad-dht#784 (currently referencing this commit directly)

@MarcoPolo MarcoPolo mentioned this pull request Jul 5, 2022
Merged
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 6, 2022 •
edited
Loading

Codecov Report

Merging #8970 (daf1731) into master (75d78de) will increase coverage by 5.45%.
The diff coverage is 29.09%.

❗ Current head daf1731 differs from pull request most recent head b5caf08. Consider uploading reports for the commit b5caf08 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8970      +/-   ##
==========================================
+ Coverage   35.19%   40.65%   +5.45%     
==========================================
  Files         703      707       +4     
  Lines       78466    78671     +205     
==========================================
+ Hits        27620    31987    +4367     
+ Misses      45851    41209    -4642     
- Partials     4995     5475     +480
Impacted Files Coverage Δ
api/types.go 52.77% <ø> (ø)
node/impl/net/rcmgr.go 0.00% <0.00%> (ø)
node/modules/lp2p/smux.go 9.09% <ø> (ø)
node/modules/lp2p/rcmgr.go 25.65% <42.10%> (-7.68%) ⬇️
chain/events/state/fastapi.go 75.00% <0.00%> (-25.00%) ⬇️
storage/pipeline/currentdealinfo.go 71.42% <0.00%> (-4.77%) ⬇️
storage/wdpost/wdpost_run_faults.go 68.30% <0.00%> (-2.82%) ⬇️
chain/consensus/filcns/mine.go 66.66% <0.00%> (-2.39%) ⬇️
storage/sealer/sched_assigner_common.go 78.40% <0.00%> (-2.28%) ⬇️
... and 149 more

@jennijuju
Copy link
Copy Markdown
Member

jennijuju commented Aug 8, 2022 •
edited
Loading

@MarcoPolo thank you for opening the draft PR! We noticed that https://github.com/libp2p/go-libp2p/releases/tag/v0.21.0 is now published, im wondering if you are planning to update this branch to the stable release and open PR for review?

it'd be great if we can have rcmgr autoscale in lotus & and have lotus nodes exposes opencensus metrics as well

@MarcoPolo
Copy link
Copy Markdown
Contributor Author

MarcoPolo commented Aug 11, 2022

@MarcoPolo thank you for opening the draft PR! We noticed that https://github.com/libp2p/go-libp2p/releases/tag/v0.21.0 is now published, im wondering if you are planning to update this branch to the stable release and open PR for review?

it'd be great if we can have rcmgr autoscale in lotus & and have lotus nodes exposes opencensus metrics as well

Yup! planning on doing that today :)

I'll probably split this into two PRs. One with just the dep updates and the other that enables the opencensus metrics.

@MarcoPolo MarcoPolo force-pushed the update-go-libp2p-v0.21 branch from b5caf08 to 6ae75aa Compare August 11, 2022 18:25
@MarcoPolo
Copy link
Copy Markdown
Contributor Author

MarcoPolo commented Aug 11, 2022

We should merge filecoin-project/go-fil-markets#744 first and we can update the go-fil-markets dep here as well.

@MarcoPolo MarcoPolo marked this pull request as ready for review August 11, 2022 20:43
@MarcoPolo MarcoPolo requested a review from a team as a code owner August 11, 2022 20:43
@jennijuju
Copy link
Copy Markdown
Member

jennijuju commented Aug 11, 2022

thanks @MarcoPolo

pinged market maintainer for t he review there, tho we probably should do separate PR for updating the market version!

@jennijuju jennijuju changed the title chore: deps: update go-libp2p to v0.21-RC chore: deps: update go-libp2p to v0.21 Aug 11, 2022
@jennijuju jennijuju requested a review from vyzo August 11, 2022 21:04
@MarcoPolo
Copy link
Copy Markdown
Contributor Author

MarcoPolo commented Aug 12, 2022

thanks @MarcoPolo

pinged market maintainer for t he review there, tho we probably should do separate PR for updating the market version!

We need to update go-fil-markets before merging this PR since there was a breaking change in go-libp2p-core and the current go-fil-markets wouldn't work with this PR since it updates go-libp2p-core.

You get this error if you try:

go: finding module for package github.com/libp2p/go-libp2p-core/mux
github.com/filecoin-project/lotus/node imports
        github.com/filecoin-project/go-fil-markets/retrievalmarket/network imports
        github.com/libp2p/go-libp2p-core/mux: module github.com/libp2p/go-libp2p-core@latest found (v0.19.1), but does not contain package github.com/libp2p/go-libp2p-core/mux

MarcoPolo
MarcoPolo commented Aug 12, 2022
View reviewed changes
node/modules/lp2p/rcmgr.go Outdated
if err != nil {
return nil, fmt.Errorf("error creating resource manager stats reporter: %w", err)
}
view.Register(obs.DefaultViews...)
Copy link
Copy Markdown
Contributor Author

@MarcoPolo MarcoPolo Aug 12, 2022

Choose a reason for hiding this comment

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

This registers the resource managers opencensus metrics

@jennijuju
Copy link
Copy Markdown
Member

jennijuju commented Aug 12, 2022

there was a breaking change

ah- ack!

@hannahhoward hannahhoward mentioned this pull request Aug 17, 2022
Merged
6 tasks
@jacobheun
Copy link
Copy Markdown
Contributor

jacobheun commented Aug 18, 2022

It looks like go-fil-markets and dht had releases for this change, what else is needed here? We have some overlap with this changeset on Boost.

magik6k
magik6k approved these changes Aug 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Looks good.

Will merge with tagged markets

@magik6k magik6k merged commit ba67431 into filecoin-project:master Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@magik6k magik6k magik6k approved these changes

@vyzo vyzo Awaiting requested review from vyzo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MarcoPolo @jennijuju @jacobheun @magik6k