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

Jump to bottom

fix(node/store): substantial memory usage improvements #2960

Merged
merged 62 commits into from
Nov 30, 2023
Merged

fix(node/store): substantial memory usage improvements #2960

merged 62 commits into from
Nov 30, 2023

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Nov 26, 2023 •
edited
Loading

  • Fixes memory leak OOM on Light and Bridge nodes #2905
  • Reduces memory consumption (from up-to 3gib to up-to 300mib) (TODO: proofs in pyroscope)
  • Improves cache hit rate on Badger
    • The warning log with Badger's low cache hit rate has disappeared.

NOTE: More details in code comments

TODO:

  • Test with lower block cache size (default 256mib)
  • Test sync on BN (@renaynay, could you try your BN?)
  • Submit a minor bug fix to libp2p(peerstoreds cyclicBatch)
  • Extract unrelated changes to a diff PRs
  • Check if this is breaking
  • Make an issue about relaxed badger configuration node: provide relaxed BadgerDB config for FN/BN #2977
  • Docs. Reduce hardware requirements, introduce new build options

Closes #2905

@Wondertan
bump badger
edfe0c2
@Wondertan
pyroscope add goroutines profiles
5e82d8f
@Wondertan
disable conflict detection
319a117
@Wondertan
gc more often
2d11f91
@Wondertan
increase discard ratio
db710eb
@Wondertan
disable compression and decrease memtable size
d2054a5
@Wondertan
further fine tuning
b621368
@Wondertan
further fine tuning
335b2b9
@Wondertan
further fine tuning
e4f37ae
@Wondertan
further fine tuning
ae53a67
@Wondertan
further fine tuning
621a241
@Wondertan
further fine tuning
b551ab5
@Wondertan
further fine tuning
d500b37
@Wondertan
further fine tuning
fdd3caa
@Wondertan
further fine tuning
2c66d22
@Wondertan
further fine tuning
069d5ca
@Wondertan
further fine tuning
fa3a7eb
@Wondertan
further fine tuning
239f3ec
@Wondertan
enable compression back
2acf2bf
@Wondertan
enable compression back
42887fa
@Wondertan
disable compression
baa7b4c
@Wondertan
debug badger
c7ee6a8
@Wondertan
debug badger more
ddf166f
@Wondertan
debug badger more
b8eed00
@Wondertan
debug badger more
3748f6d
@Wondertan
debug badger more
37c4a3d
@Wondertan
actual fix?
dc5b6ec
@Wondertan
actual fix without debug logs?
927d378
@Wondertan
inverted index improvements
0f019f5
@Wondertan
get conflict detection back
1ceed5f
@Wondertan Wondertan mentioned this pull request Nov 29, 2023
Merged
@Wondertan Wondertan mentioned this pull request Nov 29, 2023
Open
Wondertan added a commit that referenced this pull request Nov 29, 2023
@Wondertan
chore(node): tweak pyroscope options (#2974)
7fe691e 
* Increases UploadRate to 15 secs.
* Send goroutines profiles additionally 

Extracted from #2960
Wondertan added a commit that referenced this pull request Nov 29, 2023
@Wondertan
chore(modheader): lower store cache size for LN (#2975)
24b333d 
Part of an effort to decrease the memory footprint for LNs. Headers are pretty beefy.

Extracted from #2960
walldiss
walldiss reviewed Nov 30, 2023
View reviewed changes
nodebuilder/store.go Show resolved Hide resolved
nodebuilder/store.go Show resolved Hide resolved
nodebuilder/store.go Show resolved Hide resolved
renaynay
renaynay reviewed Nov 30, 2023
View reviewed changes
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Needs lint otherwise i'm fine to RC + deploy to arabica

share/eds/inverted_index.go Outdated Show resolved Hide resolved
ramin
ramin previously approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@ramin ramin left a comment

Choose a reason for hiding this comment

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

seems clear. Only comments, perpetually, it'd be nice to do a little refactoring to be able to add some tests cases around some of the behavior (eg: the NumCPU setting that is then validated as compactors > opts.MaxLevels etc) but happy to add that later

Makefile Show resolved Hide resolved
nodebuilder/header/config.go Outdated Show resolved Hide resolved
walldiss
walldiss previously approved these changes Nov 30, 2023
View reviewed changes
renaynay
renaynay previously approved these changes Nov 30, 2023
View reviewed changes
ramin
ramin previously approved these changes Nov 30, 2023
View reviewed changes
@Wondertan Wondertan dismissed stale reviews from ramin and renaynay via dd4960a November 30, 2023 15:30
@Wondertan Wondertan enabled auto-merge (squash) November 30, 2023 15:49
@Wondertan Wondertan merged commit fa6a137 into main Nov 30, 2023
29 of 38 checks passed
@Wondertan Wondertan deleted the badger-bump branch November 30, 2023 16:22
@faddat faddat mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@walldiss walldiss walldiss approved these changes

@renaynay renaynay renaynay approved these changes

@ramin ramin ramin left review comments

@vgonkivs vgonkivs Awaiting requested review from vgonkivs vgonkivs is a code owner

@distractedm1nd distractedm1nd Awaiting requested review from distractedm1nd distractedm1nd is a code owner

Assignees

@Wondertan Wondertan

Labels
area:node Node kind:fix Attached to bug-fixing PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OOM on Light and Bridge nodes
5 participants
@Wondertan @codecov-commenter @ramin @walldiss @renaynay