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

Sensu Enterprise HEAP_SIZE is not configurable #767

Closed
cwjohnston opened this issue Jul 25, 2017 · 7 comments
Closed

Sensu Enterprise HEAP_SIZE is not configurable #767

cwjohnston opened this issue Jul 25, 2017 · 7 comments
Assignees

Comments

@cwjohnston
Copy link
Contributor

Expected Behavior

When using Sensu Enterprise, this module should manage the HEAP_SIZE environment variable defined in /etc/default/sensu-enterprise

Current Behavior

This module does not expose a parameter for managing HEAP_SIZE

Context

Attempting to manage the contents of /etc/default/sensu-enterprise or /etc/default/sensu outside of this module can lead to unnecessary service restarts.

Possible Solution

Add a parameter to sensu class providing control over the HEAP_SIZE when the value of the sensu_enterprise parameter is true

@cwjohnston
Copy link
Contributor Author

Note that it's equally reasonable to configure this environment variable via /etc/default/sensu, as it will also be evaluated when starting sensu-enterprise

@alvagante
Copy link
Collaborator

@cwjohnston please correct me if I'm wrong:
I'm going to add a heap_size param that, if defined, adds to /etc/default/sensu (or /etc/default/sensu-enterprise if used) the line:
HEAP_SIZE=<%= @heap_size %>
I suspect the value might not only be an Integer (ie 256M) so validation should be for a normal string.

Please confirm or correct me.

@cwjohnston
Copy link
Contributor Author

@alvagante I believe you are correct, HEAP_SIZE should allow a string value so the suffix can indicate unit of measure.

FWIW, HEAP_SIZE has no effect on Sensu Core (sensu-server), only on Sensu Enterprise.

alvagante added a commit to alvagante/sensu-puppet that referenced this issue Jul 26, 2017
@alvagante
Copy link
Collaborator

@cwjohnston thanks for quick reply. The commit (blind code, still not tested on real vm) done modifies the file for both, as the same template is used.
If you want I can remove the changes in alvagante@7f706f6#diff-1576180ba8964fff8d837b3256127ebc so that in sensu-core even if heap_size is set, it does not appear in /etc/default/sensu.
Alternatively I would add a note in the docs and leave the heap_size param in sensu::package as is. Eventually HEAP_SIZE might be used in the future also in sensu-core.

Any preference?

alvagante added a commit to alvagante/sensu-puppet that referenced this issue Jul 26, 2017
alvagante added a commit to alvagante/sensu-puppet that referenced this issue Jul 26, 2017
Added heap_size param sensu#767

Fixes and notes

Added spec tests for sensu#767

Minor
@ghoneycutt
Copy link
Collaborator

Prefer to keep it as a param in init.pp and just document that it is only for the enterprise software.

@alvagante
Copy link
Collaborator

@ghoneycutt so is the current implementation, even if it has a redundant parameter in sensu::package.
If you want I can remove sensu::package::heap_size, and relevant tests, otherwise #771 should be ready to merge

ghoneycutt added a commit that referenced this issue Jul 26, 2017
@ghoneycutt
Copy link
Collaborator

Released in v2.30.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants