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 timeout_stop to control systemd TimeoutStopSec #499

Closed

Conversation

antondollmaier
Copy link

@antondollmaier antondollmaier commented Oct 3, 2020

Pull Request (PR) description

With larger WAL segments, prometheus fails to write
a new checkpoint in TimeoutStopSec time:

Oct 03 15:13:37 prometheus prometheus[2452]: level=info ts=2020-10-03T15:13:37.751Z caller=checkpoint.go:96 component=tsdb msg="Creating checkpoint" from_segment=85417 to_segment=85677 mint
Oct 03 15:15:06 prometheus systemd[1]: prometheus.service: State 'stop-sigterm' timed out. Killing.
Oct 03 15:15:06 prometheus systemd[1]: prometheus.service: Killing process 2452 (prometheus) with signal SIGKILL.
Oct 03 15:15:06 prometheus systemd[1]: prometheus.service: Main process exited, code=killed, status=9/KILL
Oct 03 15:15:06 prometheus systemd[1]: prometheus.service: Failed with result 'timeout'.
Oct 03 15:15:06 prometheus systemd[1]: Stopped Prometheus Monitoring framework.

This change adds a new parameter to define TimeoutStopSec
for prometheus.service.

This Pull Request (PR) fixes the following issues

No issue previously created.

The required systemd unitfile overwrite could have been made locally as well - but as I suppose that others could also face similar issues, I decided to implement the parameter directly.

@antondollmaier antondollmaier force-pushed the timeoutstopsec branch 2 times, most recently from e4e4231 to b2fab21 Compare October 3, 2020 15:42
With larger WAL segments, prometheus fails to write
a new checkpoint in `TimeoutStopSec` time.

This change adds a new parameter to define `TimeoutStopSec`
for `prometheus.service`.
@antondollmaier antondollmaier marked this pull request as ready for review October 3, 2020 16:09
Copy link

@igalic igalic left a comment

Choose a reason for hiding this comment

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

🙋🏻‍♀️

@@ -397,6 +397,16 @@ Currently only implemented for systemd based service.

Default value: ``undef``

##### `timeout_stop`

Data type: `Optional[String]`
Copy link

Choose a reason for hiding this comment

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

i feel like we can do better than this
but that better type would either belong here:
https://github.com/camptocamp/puppet-systemd/tree/master/types

unless a general one already exists here: https://github.com/puppetlabs/puppetlabs-stdlib/tree/main/types

Copy link
Author

Choose a reason for hiding this comment

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

I totally agree that "String" is very ... liberal - and I checked both modules before.

Let's see if there's nice regex to be drafted in a puppet type before merging.

@bastelfreak bastelfreak added the enhancement New feature or request label Dec 4, 2020
@bastelfreak
Copy link
Member

@antondollmaier can you please rebase against our latest master branch?

@bastelfreak
Copy link
Member

@antondollmaier ping :)

@vox-pupuli-tasks
Copy link

Dear @antondollmaier, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@TheMeier
Copy link
Contributor

TheMeier commented Jun 3, 2024

I have opened a new issue to change the systemd configuration, which will allow customization like this without adding new parameters. #734
Since this PR has not seen any action for a while I will close it.

@TheMeier TheMeier closed this Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants