Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: only enable rcmgr by default in full nodes #8769

Merged
merged 4 commits into from
May 31, 2022
Merged

feat: only enable rcmgr by default in full nodes #8769

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a313dcb
only enable rcmgr by default in full nodes
vyzo May 31, 2022
a1e6410
remove outdated comment
vyzo May 31, 2022
bc180c8
update comment.
vyzo May 31, 2022
207ff66
update comment.
vyzo May 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions node/modules/lp2p/rcmgr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ import (

func ResourceManager(connMgrHi uint) func(lc fx.Lifecycle, repo repo.LockedRepo) (network.ResourceManager, error) {
return func(lc fx.Lifecycle, repo repo.LockedRepo) (network.ResourceManager, error) {
isFullNode := repo.RepoType().Type() == "FullNode"
envvar := os.Getenv("LOTUS_RCMGR")
if envvar == "0" {
// this is enabled by default; specify LOTUS_RCMGR=0 to disable
if (isFullNode && envvar == "0") || // enable by default for full nodes
Copy link
Collaborator

@ribasushi ribasushi May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Logic seems wrong, I think it should be IGNORE, I misread the outer logic of this:

Suggested change
if (isFullNode && envvar == "0") || // enable by default for full nodes
if (isFullNode && envvar != "0") || // enable by default for full nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the branch is for disabling, logic is correct.
Maybe fix the comment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I retract comment, the entire if block is to do network.NullResourceManager, I am a muppet ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we might want to fix the comment to avoid tripping casual reading.
Any suggestions?

vyzo marked this conversation as resolved.
Show resolved Hide resolved
(!isFullNode && envvar != "1") { // otherwise, opt-in with envvar
vyzo marked this conversation as resolved.
Show resolved Hide resolved
log.Info("libp2p resource manager is disabled")
return network.NullResourceManager, nil
}
Expand Down
4 changes: 4 additions & 0 deletions node/repo/fsrepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,10 @@ type fsLockedRepo struct {
configLk sync.Mutex
}

func (fsr *fsLockedRepo) RepoType() RepoType {
return fsr.repoType
}

func (fsr *fsLockedRepo) Readonly() bool {
return fsr.readonly
}
Expand Down
3 changes: 3 additions & 0 deletions node/repo/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ type LockedRepo interface {
// Close closes repo and removes lock.
Close() error

// returns the type of this repo
RepoType() RepoType

// Returns datastore defined in this repo.
// The supplied context must only be used to initialize the datastore.
// The implementation should not retain the context for usage throughout
Expand Down
4 changes: 4 additions & 0 deletions node/repo/memrepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ type lockedMemRepo struct {
token *byte
}

func (lmem *lockedMemRepo) RepoType() RepoType {
return lmem.t
}

func (lmem *lockedMemRepo) GetStorage() (stores.StorageConfig, error) {
if err := lmem.checkToken(); err != nil {
return stores.StorageConfig{}, err
Expand Down