grafana / grafana/alerting

alertmanager: dispatcher restart causes partial group notifications due to non-deterministic alert ordering

Open
#555 0 comments 0 reactions 1 assignee Claimed by @yuri-tceretian View on GitHub
bug
Dominant language
Go
Stars
89
Forks
74
Avg merge
2d 7h
Merged PRs (30d)
8

Description

## Summary

When the alertmanager dispatcher restarts (e.g. due to a config reload), it re-seeds its alert queue from `mem.Alerts` via `Subscribe()`. Because `mem.Alerts.List()` iterates a Go `map`, alert ordering is non-deterministic. Combined with the `insert()` early-flush logic that fires when `StartsAt + GroupWait < now`, the first alert processed can flush immediately — before any sibling alerts from the same aggregation group are inserted — producing a notification that contains only a subset of the group's alerts.

## Affected Code

**`vendor/github.com/prometheus/alertmanager/store/store.go:132-142`**

```go
func (a *Alerts) List() []*types.Alert {
a.Lock()
defer a.Unlock()

alerts := make([]*types.Alert, 0, len(a.c))
for _, alert := range a.c { // map iteration — random order
alerts = append(alerts, alert)
}
return alerts
}
```

**`vendor/github.com/prometheus/alertmanager/dispatch/dispatch.go:536-548`**

```go
func (ag *aggrGroup) insert(alert *types.Alert) {
if err := ag.alerts.Set(alert); err != nil { ... }

ag.mtx.Lock()
defer ag.mtx.Unlock()
if !ag.hasFlushed && alert.StartsAt.Add(ag.opts.GroupWait).Before(time.Now()) {
ag.timer.Flush() // fires immediately, before siblings are inserted
}
}
```

## Reproduction Scenario

1. Two alerts (`A` and `B`) belong to the same aggregation group, both firing for longer than `group_wait`.
2. A config reload restarts the dispatcher. `ApplyConfig` discards aggrGroup state (by design).
3. The new dispatcher calls `Subscribe()` → `mem.Alerts.List()` → random map order.
4. If alert `A` is returned first:
- `processAlert(A)` creates a new aggrGroup, calls `insert(A)`.
- `StartsAt(A) + group_wait < now` → `timer.Flush()` → `go ag.run()` launched.
- `ag.run()` fires immediately, calling `flush()` → `ag.alerts.List()` returns only `{A}`.
5. Alert `B` is inserted next, 1–2ms later. `hasFlushed` is already `true` → no early flush, `B` waits for the next `group_interval` tick.
6. Result: notification sent with only `A`. `B` notified separately at `group_interval`.

## Observed Impact

In a production incident, this produced two separate Slack notifications ~2 minutes apart for alerts that started firing simultaneously and belong to the same group. A correct grouped notification had fired immediately before the reload. After the reload, each alert in the group was notified individually across successive dispatcher restarts.

## Proposed Fix

Make `store.List()` return alerts in a **deterministic order** (e.g. sorted by fingerprint). This ensures sibling alerts within the same group are always processed in a consistent sequence.

```go
func (a *Alerts) List() []*types.Alert {
a.Lock()
defer a.Unlock()

alerts := make([]*types.Alert, 0, len(a.c))
for _, alert := range a.c {
alerts = append(alerts, alert)
}
sort.Slice(alerts, func(i, j int) bool {
return alerts[i].Fingerprint() < alerts[j].Fingerprint()
})
return alerts
}
```

This removes the map-randomness amplifier and makes the behaviour deterministic. A more complete fix would batch-insert all alerts seeded from `Subscribe()` into their aggrGroups before allowing any early flush, but that requires deeper changes to the dispatch pipeline.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.