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

Alerting: Fix migration to create rules with group index 1 #56511

Merged
merged 2 commits into from Oct 7, 2022

Conversation

yuri-tceretian
Copy link
Contributor

What this PR does / why we need it:
Currently, during migration Grafana creates new records in table alert_rule with value 0 in field rule_group_idx (default). Then during the next migration steps, Grafana reads the rules with the value 0 in that field and updates it to 1, and also creates a record in table alert_rule_version.
This can cause another problem: the migration can fail if there are too many records are inserted into the version table because they are inserted in bulk, and it causes (a bug?) error in XORM "Prepared statement contains too many placeholders".

This PR

  1. fixes the migration to set field rule_group_idx to 1 by default, to avoid unnecessary operations.
  2. Updates the command that inserts records to table alert_rule_version to do this by a single row.

@yuri-tceretian yuri-tceretian added the area/alerting Grafana Alerting label Oct 6, 2022
@yuri-tceretian yuri-tceretian added this to the 9.1.8 milestone Oct 6, 2022
@yuri-tceretian yuri-tceretian self-assigned this Oct 6, 2022
@yuri-tceretian yuri-tceretian requested a review from a team as a code owner October 6, 2022 21:18
@yuri-tceretian yuri-tceretian added add to changelog backport v9.1.x Bot will automatically open backport PR backport v9.2.x Mark PR for automatic backport to v9.2.x labels Oct 6, 2022
Copy link
Contributor

@santihernandezc santihernandezc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

pkg/services/sqlstore/migrations/ualert/alert_rule.go Outdated Show resolved Hide resolved
@yuri-tceretian yuri-tceretian merged commit 3487e68 into grafana:main Oct 7, 2022
@yuri-tceretian yuri-tceretian deleted the migration-bugs branch October 7, 2022 21:20
grafanabot pushed a commit that referenced this pull request Oct 7, 2022
grafanabot pushed a commit that referenced this pull request Oct 7, 2022
yuri-tceretian added a commit that referenced this pull request Oct 7, 2022
…56583)

(cherry picked from commit 3487e68)

Co-authored-by: Yuriy Tseretyan <yuriy.tseretyan@grafana.com>
yuri-tceretian added a commit that referenced this pull request Oct 7, 2022
…56584)

(cherry picked from commit 3487e68)

Co-authored-by: Yuriy Tseretyan <yuriy.tseretyan@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/alerting Grafana Alerting area/backend/db/migration area/backend backport v9.1.x Bot will automatically open backport PR backport v9.2.x Mark PR for automatic backport to v9.2.x
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants