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.

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

Added memory ballast flag #3488

Merged
merged 9 commits into from
Oct 8, 2021
Merged

Added memory ballast flag #3488

merged 9 commits into from
Oct 8, 2021

Conversation

bogdan-rosianu
Copy link
Contributor

added a flag for memory ballast optimization

@bogdan-rosianu bogdan-rosianu added the type:feature New feature or request label Oct 1, 2021
@bogdan-rosianu bogdan-rosianu self-assigned this Oct 1, 2021
@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #3488 (3b33041) into development (78baaba) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #3488   +/-   ##
============================================
  Coverage        73.88%   73.88%           
============================================
  Files              582      582           
  Lines            74444    74444           
============================================
  Hits             55000    55000           
  Misses           15044    15044           
  Partials          4400     4400           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27677e4...3b33041. Read the comment docs.

// memory ballast is an optimization for golang's garbage collector. If set to a high value, it can decrease
// the number of times when GC performs STW processes, that results is a better performance over high load
memoryBallastObject = make([]byte, memBallastValue*core.MegabyteSize)
log.Debug("initialized memory ballast object", "size", core.ConvertBytes(uint64(len(memoryBallastObject))))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the compiler might optimize the code and remove the memBallastValue after this call. Might add a new print on L132 that uses memBallastValue. The node might call runtime.GC after shuffling out a shard. This call should theoretically remove the memoryBallastObject object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added some prints and tested locally. The object won't be removed/freed

sasurobert
sasurobert previously approved these changes Oct 1, 2021
iulianpascalau
iulianpascalau previously approved these changes Oct 4, 2021
@@ -305,7 +305,9 @@ var (
Name: "mem-ballast",
Value: 0,
Usage: "Flag that specifies the number of MegaBytes to be used as a memory ballast for Garbage Collector optimization. " +
"If set to 0, the feature will be disabled",
"If set to 0 (or not set at all), the feature will be disabled. This flag should be used only for well-monitored nodes " +
"and by advanced users, as a too high memory ballast could lead to Out Of Memory panics. The memory ballast " +
Copy link
Contributor

Choose a reason for hiding this comment

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

"because of golang" 🤦

Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

System test passed.
image

@LucianMincu LucianMincu merged commit a9ff826 into development Oct 8, 2021
@LucianMincu LucianMincu deleted the add-memory-ballast-flag branch October 8, 2021 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants