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 LimitNOFILE to prometheus server unit #297

Closed
wants to merge 3 commits into from

Conversation

sc0rp10
Copy link

@sc0rp10 sc0rp10 commented Feb 16, 2019

As a Brian Brazil said nofile limit should be set to an large value, one billion as example.
I think that we should set this in service unit because of any other ways may not work unlike systemd.

@bastelfreak
Copy link
Member

HI @sc0rp10, thanks for the PR. I don't think that such a high value is a good default. Could you make it configureable and optional?

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet tests-fail labels Feb 17, 2019
@sc0rp10
Copy link
Author

sc0rp10 commented Feb 17, 2019

Hi, @bastelfreak I was guided by Brian's article that I linked earlier, what do you think about it?

@bastelfreak
Copy link
Member

This value is way to high for small installations. So I think it should be an optional parameter.

@sc0rp10
Copy link
Author

sc0rp10 commented Feb 17, 2019

what default value are you prefer?

@alexjfisher
Copy link
Member

I think the idea proposed is that you make it an Optional[Integer] parameter that defaults to undef. Then in the template you only write the line if the value was set to something.

@bastelfreak
Copy link
Member

Ping @sc0rp10 :)

@dhoppe
Copy link
Member

dhoppe commented Jun 5, 2019

@bastelfreak I would like to close this pull request in favor of #298. Any objections?

@bastelfreak bastelfreak closed this Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants