Skip to content

Fix MaxSilenceSizeBytes limit causes incomplete updates of existing silences#3897

Merged
gotjosh merged 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/fix-limits-replacing-silences-2
Jun 25, 2024
Merged

Fix MaxSilenceSizeBytes limit causes incomplete updates of existing silences#3897
gotjosh merged 1 commit intoprometheus:mainfrom
grobinson-grafana:grobinson/fix-limits-replacing-silences-2

Conversation

@grobinson-grafana
Copy link
Collaborator

@grobinson-grafana grobinson-grafana commented Jun 24, 2024

This commit fixes a bug where the MaxSilenceSizeBytes limit can cause an incomplete update of existing silences, where the old silence can be expired but the new silence cannot be created because it would exceed the maximum size limit.

I moved sil.UpdatedAt = now out of setSilence and into Set and Expire because we want to make sure the UpdatedAt timestamp is calculated in the size when we call checkSizeLimits. Forget to set sil.UpdatedAt in either of these places and the following tests fail:

--- FAIL: TestSilenceSet (0.00s)
--- FAIL: TestSilenceLimits (0.00s)
--- FAIL: TestSetActiveSilence (0.00s)
--- FAIL: TestSilenceExpire (0.00s)
--- FAIL: TestSilenceExpireWithZeroRetention (0.00s)
--- FAIL: TestSilenceExpireInvalid (0.00s)

@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-limits-replacing-silences-2 branch 2 times, most recently from 0ea534d to 4c272b7 Compare June 24, 2024 15:33
@grobinson-grafana grobinson-grafana marked this pull request as draft June 24, 2024 16:11
@grobinson-grafana grobinson-grafana marked this pull request as draft June 24, 2024 16:11
@grobinson-grafana grobinson-grafana changed the title Fix other incomplete updates of existing silences Fix MaxSilenceSizeBytes limit causes incomplete updates of existing silences Jun 24, 2024
@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-limits-replacing-silences-2 branch 2 times, most recently from 6d94575 to 698f3b2 Compare June 24, 2024 17:23
@grobinson-grafana grobinson-grafana marked this pull request as ready for review June 24, 2024 17:24
@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-limits-replacing-silences-2 branch 6 times, most recently from caf0a48 to fa74725 Compare June 25, 2024 11:56
…ilences

This commit fixes a bug where the MaxSilenceSizeBytes limit can
cause an incomplete update of existing silences, where the old
silence can be expired but the new silence cannot be created
because it would exceed the maximum size limit.

Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-limits-replacing-silences-2 branch from fa74725 to c3f442c Compare June 25, 2024 12:04
@gotjosh gotjosh merged commit 94ac36b into prometheus:main Jun 25, 2024
grobinson-grafana added a commit to grobinson-grafana/alertmanager that referenced this pull request Jun 25, 2024
…ilences (prometheus#3897)

This commit fixes a bug where the MaxSilenceSizeBytes limit can
cause an incomplete update of existing silences, where the old
silence can be expired but the new silence cannot be created
because it would exceed the maximum size limit.

Signed-off-by: George Robinson <george.robinson@grafana.com>
@grobinson-grafana grobinson-grafana deleted the grobinson/fix-limits-replacing-silences-2 branch June 25, 2024 15:59
grobinson-grafana added a commit to grafana/mimir that referenced this pull request Jun 26, 2024
This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.
grobinson-grafana added a commit to grafana/mimir that referenced this pull request Jun 26, 2024
This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.
grobinson-grafana added a commit to grafana/mimir that referenced this pull request Jun 26, 2024
This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.

additional tests in upstream.
grobinson-grafana added a commit to grafana/mimir that referenced this pull request Jun 26, 2024
This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.
dimitarvdimitrov pushed a commit to grafana/mimir that referenced this pull request Jul 2, 2024
This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.

(cherry picked from commit 1cfb657)
dimitarvdimitrov added a commit to grafana/mimir that referenced this pull request Jul 2, 2024
* Fixes a number of bugs in silences (#8525)

This commit fixes the following bugs in silences:

- prometheus/alertmanager#3877
- prometheus/alertmanager#3898
- prometheus/alertmanager#3897

which could cause an existing silence to be deleted/expired
when updating the silence failed. This could be because
the replacing silence exceeded limits or was invalid.

(cherry picked from commit 1cfb657)

* Update CHANGELOG.md (#8526)

(cherry picked from commit 36f7af3)

---------

Co-authored-by: George Robinson <george.robinson@grafana.com>
TheMeier pushed a commit to TheMeier/alertmanager that referenced this pull request Sep 29, 2024
…ilences (prometheus#3897)

This commit fixes a bug where the MaxSilenceSizeBytes limit can
cause an incomplete update of existing silences, where the old
silence can be expired but the new silence cannot be created
because it would exceed the maximum size limit.

Signed-off-by: George Robinson <george.robinson@grafana.com>
tjhop pushed a commit to tjhop/alertmanager that referenced this pull request Nov 13, 2025
…ilences (prometheus#3897)

This commit fixes a bug where the MaxSilenceSizeBytes limit can
cause an incomplete update of existing silences, where the old
silence can be expired but the new silence cannot be created
because it would exceed the maximum size limit.

Signed-off-by: George Robinson <george.robinson@grafana.com>
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

Successfully merging this pull request may close these issues.

2 participants