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.

Sign up for GitHub

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

Jump to bottom

Update Grafana ClickHouse alerting #229

Merged
merged 2 commits into from Feb 6, 2020
Merged

Update Grafana ClickHouse alerting #229

merged 2 commits into from Feb 6, 2020

Conversation

Aquatoid87
Copy link
Contributor

Hi, @andyone , we have updated ClickHouse alerting for Grafana, it will be nice to check this PR, and to build RPM-package from spec.

@Aquatoid87 Aquatoid87 changed the base branch from master to develop December 24, 2019 12:16
@andyone andyone self-assigned this Dec 24, 2019
@gongled
Copy link
Contributor

gongled commented Dec 24, 2019

@andyone, keep in mind, that @Aquatoid87 also put systemd unit file to change Restart parameter from on-failure (by default) to always to let process automatically restart after database connection loss.

@andyone
Copy link
Member

andyone commented Dec 24, 2019

@Aquatoid87 Thanks for your contribution.

@gongled Yep, I have noticed that. I think if we set Restart to always, StartLimitIntervalSec and StartLimitBurst also should be set.

andyone
andyone requested changes Dec 24, 2019
View reviewed changes
specs/grafana/SOURCES/grafana-server.service
User=grafana
Group=grafana
Type=notify
Restart=always
Copy link
Member

Choose a reason for hiding this comment

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

Please add StartLimitIntervalSec and StartLimitBurst.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed (4ab1291)

specs/grafana/SOURCES/grafana-server.service Outdated
Documentation=http://docs.grafana.org
Wants=network-online.target
After=network-online.target
After=postgresql.service mariadb.service mysqld.service
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add clickhouse-server.service here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed (4ab1291)

@andyone andyone self-requested a review February 6, 2020 10:18
@andyone
Copy link
Member

andyone commented Feb 6, 2020

Looks good for me now.

@gongled @Aquatoid87 Pay attention that you should look after your patches for this package. All outdated patches will be removed at the moment of the package update.

@andyone andyone merged commit 219568b into essentialkaos:develop Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gongled gongled gongled left review comments

@andyone andyone andyone approved these changes

Assignees

@andyone andyone

Labels
PR • IMPROVEMENT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Aquatoid87 @gongled @andyone