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

Add new gc_max_allocs tuneable #2636

Merged
merged 6 commits into from
May 30, 2017
Merged

Add new gc_max_allocs tuneable #2636

merged 6 commits into from
May 30, 2017

Conversation

schmichael
Copy link
Member

@schmichael schmichael commented May 12, 2017

More than gc_max_allocs may be running on a node, but terminal allocs
will be garbage collected to try to keep the total number below the
limit.

The default of 200 allocs was a guess. Let me know if you have a number with more science behind it.

I also split go gc.run() out of the NewAllocGarbageCollector function as tests were creating and leaking gc goroutines. Not a big deal, but since it's only called one place in runtime code it's not really any convenience to hide it away inside the New func.

@schmichael schmichael requested a review from dadgar May 12, 2017 00:16
More than gc_max_allocs may be running on a node, but terminal allocs
will be garbage collected to try to keep the total number below the
limit.
Copy link
Contributor

@dadgar dadgar left a comment

Choose a reason for hiding this comment

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

Great job!

@@ -205,6 +209,7 @@ func DefaultConfig() *Config {
GCParallelDestroys: 2,
GCDiskUsageThreshold: 80,
GCInodeUsageThreshold: 70,
GCMaxAllocs: 200,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest 50

@@ -100,6 +100,12 @@ client {
- `gc_inode_usage_threshold` `(float: 70)` - Specifies the inode usage percent
which Nomad tries to maintain by garbage collecting terminal allocations.

- `gc_max_allocs` `(int: 200)` - Specifies the maximum number of allocations
Copy link
Contributor

Choose a reason for hiding this comment

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

Update to 50

@dadgar
Copy link
Contributor

dadgar commented May 17, 2017

One failing config test: https://travis-ci.org/hashicorp/nomad/builds/231747600#L3543

@schmichael schmichael merged commit 236ef21 into master May 30, 2017
@schmichael schmichael deleted the f-gc-alloc-limit branch May 30, 2017 23:14
@github-actions
Copy link

github-actions bot commented Apr 1, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants