Alerting: Support concurrent queries for saving alert instances #70525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default.
grobinson-grafana
added
area/alerting
grobinson-grafana
requested review from
zserge,
mildwonkey and
suntala
and removed request for
a team,
torkelo,
zserge,
mildwonkey and
suntala
grobinson-grafana
force-pushed
the
grobinson/support-concurrent-queries-for-saving-alert-instances
branch
from
3656109
to
5669998
Compare
stevesg
approved these changes
Jun 22, 2023
There was a problem hiding this comment.
Nice change - really clean implementation.
My only nits which are non-blocking:
- Naming, I'd have gone for
max_state_save_concurrency, but that's purely subjective so I'm happy with it as-is. - A test to exercise a concurrency value other than 1, but I don't think it's worth the effort. The
ForEachJobfunction in dskit is battle tested, so I'm not especially worried.
| logger.Debug("Saving alert states done", "count", len(states)) | ||
|
|
||
| logger.Debug("Saving alert states", "count", len(states), "max_concurrent_state_savers", st.maxConcurrentStateSavers) | ||
| _ = concurrency.ForEachJob(ctx, len(states), st.maxConcurrentStateSavers, saveState) |
There was a problem hiding this comment.
Though the ForEachJob function will work as expected with concurrency=1, it will spawn a Goroutine needlessly. This is probably not anything to be worried about, just flagging it.
I like it, will change it! |
|
concurrency=1 (default): concurrency=10 |
grobinson-grafana
deleted the
grobinson/support-concurrent-queries-for-saving-alert-instances
branch
LudoVio
pushed a commit
that referenced
this pull request
Jun 26, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default.
santihernandezc
pushed a commit
that referenced
this pull request
Jun 28, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72)
santihernandezc
pushed a commit
that referenced
this pull request
Jun 28, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72)
santihernandezc
pushed a commit
that referenced
this pull request
Jun 28, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72)
santihernandezc
pushed a commit
that referenced
this pull request
Jun 28, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72)
santihernandezc
added a commit
that referenced
this pull request
Jun 29, 2023
…nces (#70869) * Alerting: Support concurrent queries for saving alert instances (#70525) This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72) * remove changes in api_testing.go --------- Co-authored-by: George Robinson <george.robinson@grafana.com>
santihernandezc
added
backport v9.5.x
|
Hello @santihernandezc!
Please, if the current pull request addresses a bug fix, label it with the |
santihernandezc
pushed a commit
that referenced
this pull request
Jun 29, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72)
harisrozajac
pushed a commit
that referenced
this pull request
Jun 29, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default.
harisrozajac
pushed a commit
that referenced
this pull request
Jun 30, 2023
This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default.
grobinson-grafana
added a commit
that referenced
this pull request
Aug 16, 2023
…ces (#70921) * Alerting: Support concurrent queries for saving alert instances (#70525) This commit adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default. (cherry picked from commit 7edbe72) * Trigger PR automation --------- Co-authored-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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this feature?
This pull request adds support for concurrent queries when saving alert instances to the database. This is an experimental feature in response to some customers experiencing delays between rule evaluation and sending alerts to Alertmanager, resulting in flapping. It is disabled by default.
1 concurrent saver (default):
10 concurrent savers:
Why do we need this feature?
[Add a description of the problem the feature is trying to solve.]
Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: