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 internal alert ingest queue #1832

Open
stuartnelson3 opened this issue Apr 11, 2019 · 1 comment
Open

Add internal alert ingest queue #1832

stuartnelson3 opened this issue Apr 11, 2019 · 1 comment

Comments

@stuartnelson3
Copy link
Contributor

Currently, every alert received via the API is sent directly to the internal memory store for alerts:

API:

if err := api.alerts.Put(validAlerts...); err != nil {

Memory store:

func (a *Alerts) Put(alerts ...*types.Alert) error {
for _, alert := range alerts {
fp := alert.Fingerprint()
// Check that there's an alert existing within the store before
// trying to merge.
if old, err := a.alerts.Get(fp); err == nil {
// Merge alerts if there is an overlap in activity range.
if (alert.EndsAt.After(old.StartsAt) && alert.EndsAt.Before(old.EndsAt)) ||
(alert.StartsAt.After(old.StartsAt) && alert.StartsAt.Before(old.EndsAt)) {
alert = old.Merge(alert)
}
}
if err := a.alerts.Set(alert); err != nil {
level.Error(a.logger).Log("msg", "error on set alert", "err", err)
continue
}
a.mtx.Lock()
for _, l := range a.listeners {
select {
case l.alerts <- alert:
case <-l.done:
}
}
a.mtx.Unlock()
}
return nil
}

Every set locks the internal store, which can become a performance issue (cf. #1201). Prometheus sends messages in batches of 50, but there's no enforcing of this on the alertmanager end. Anecdotally, I've seen a single alertmanager become unresponsive when receiving ~50 ingest requests per second, with each request containing a single message.

Batching at the store level is probably the right place to implement this:

func (a *Alerts) Set(alert *types.Alert) error {

Adding a basic benchmark and a receive queue would be a good first step.

This is more or less inspired by common sense and being reminded of how kafka ingests messages.

@palash25
Copy link

palash25 commented Nov 6, 2022

hi i would like to take this up if this is still valid and no one is working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants