From a735c51202a76e8ee63ab96d38e36d3291ce9c99 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Wed, 12 May 2021 07:17:43 -0400 Subject: [PATCH] Alerting/Chore: Backend remove def_ columns from instance (#33875) rename def_uid and def_org_id to rule_uid and rule_org_id on the alert_instance table and drops the definition table. --- pkg/services/ngalert/models/instance.go | 22 +++++-------- pkg/services/ngalert/schedule/schedule.go | 31 ++++++++++--------- pkg/services/ngalert/store/alert_rule.go | 8 ++--- pkg/services/ngalert/store/database.go | 8 ----- pkg/services/ngalert/store/database_mig.go | 21 +++++++++++++ .../ngalert/store/instance_database.go | 28 ++++++++--------- .../ngalert/tests/instance_database_test.go | 2 +- 7 files changed, 64 insertions(+), 56 deletions(-) diff --git a/pkg/services/ngalert/models/instance.go b/pkg/services/ngalert/models/instance.go index dd14f8a2bb4..f3e6ae19941 100644 --- a/pkg/services/ngalert/models/instance.go +++ b/pkg/services/ngalert/models/instance.go @@ -7,8 +7,8 @@ import ( // AlertInstance represents a single alert instance. type AlertInstance struct { - RuleOrgID int64 `xorm:"def_org_id"` - RuleUID string `xorm:"def_uid"` + RuleOrgID int64 `xorm:"rule_org_id"` + RuleUID string `xorm:"rule_uid"` Labels InstanceLabels LabelsHash string CurrentState InstanceStateType @@ -73,14 +73,10 @@ type ListAlertInstancesQuery struct { Result []*ListAlertInstancesQueryResult } -type FetchUniqueOrgIdsQuery struct { - Result []*FetchUniqueOrgIdsQueryResult -} - // ListAlertInstancesQueryResult represents the result of listAlertInstancesQuery. type ListAlertInstancesQueryResult struct { - RuleOrgID int64 `xorm:"def_org_id" json:"definitionOrgId"` - RuleDefinitionUID string `xorm:"def_uid" json:"definitionUid"` + RuleOrgID int64 `xorm:"rule_org_id" json:"ruleOrgId"` + RuleUID string `xorm:"rule_uid" json:"ruleUid"` Labels InstanceLabels `json:"labels"` LabelsHash string `json:"labeHash"` CurrentState InstanceStateType `json:"currentState"` @@ -89,11 +85,7 @@ type ListAlertInstancesQueryResult struct { LastEvalTime time.Time `json:"lastEvalTime"` } -type FetchUniqueOrgIdsQueryResult struct { - DefinitionOrgID int64 `xorm:"def_org_id" json:"definitionOrgId"` -} - -// ValidateAlertInstance validates that the alert instance contains an alert definition id, +// ValidateAlertInstance validates that the alert instance contains an alert rule id, // and state. func ValidateAlertInstance(alertInstance *AlertInstance) error { if alertInstance == nil { @@ -101,11 +93,11 @@ func ValidateAlertInstance(alertInstance *AlertInstance) error { } if alertInstance.RuleOrgID == 0 { - return fmt.Errorf("alert instance is invalid due to missing alert definition organisation") + return fmt.Errorf("alert instance is invalid due to missing alert rule organisation") } if alertInstance.RuleUID == "" { - return fmt.Errorf("alert instance is invalid due to missing alert definition uid") + return fmt.Errorf("alert instance is invalid due to missing alert rule uid") } if !alertInstance.CurrentState.IsValid() { diff --git a/pkg/services/ngalert/schedule/schedule.go b/pkg/services/ngalert/schedule/schedule.go index bc87c48e7c4..b7f2490176f 100644 --- a/pkg/services/ngalert/schedule/schedule.go +++ b/pkg/services/ngalert/schedule/schedule.go @@ -305,16 +305,19 @@ func (sch *schedule) Ticker(grafanaCtx context.Context, stateManager *state.Mana sch.registry.del(key) } case <-grafanaCtx.Done(): - err := dispatcherGroup.Wait() - orgIdsCmd := models.FetchUniqueOrgIdsQuery{} - if err := sch.instanceStore.FetchOrgIds(&orgIdsCmd); err != nil { + waitErr := dispatcherGroup.Wait() + + orgIds, err := sch.instanceStore.FetchOrgIds() + if err != nil { sch.log.Error("unable to fetch orgIds", "msg", err.Error()) } - for _, v := range orgIdsCmd.Result { - sch.saveAlertStates(stateManager.GetAll(v.DefinitionOrgID)) + + for _, v := range orgIds { + sch.saveAlertStates(stateManager.GetAll(v)) } + stateManager.Close() - return err + return waitErr } } } @@ -346,16 +349,16 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { sch.log.Info("warming cache for startup") st.ResetCache() - orgIdsCmd := models.FetchUniqueOrgIdsQuery{} - if err := sch.instanceStore.FetchOrgIds(&orgIdsCmd); err != nil { + orgIds, err := sch.instanceStore.FetchOrgIds() + if err != nil { sch.log.Error("unable to fetch orgIds", "msg", err.Error()) } var states []*state.State - for _, orgIdResult := range orgIdsCmd.Result { + for _, orgId := range orgIds { // Get Rules ruleCmd := models.ListAlertRulesQuery{ - OrgID: orgIdResult.DefinitionOrgID, + OrgID: orgId, } if err := sch.ruleStore.GetOrgAlertRules(&ruleCmd); err != nil { sch.log.Error("unable to fetch previous state", "msg", err.Error()) @@ -368,16 +371,16 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { // Get Instances cmd := models.ListAlertInstancesQuery{ - RuleOrgID: orgIdResult.DefinitionOrgID, + RuleOrgID: orgId, } if err := sch.instanceStore.ListAlertInstances(&cmd); err != nil { sch.log.Error("unable to fetch previous state", "msg", err.Error()) } for _, entry := range cmd.Result { - ruleForEntry, ok := ruleByUID[entry.RuleDefinitionUID] + ruleForEntry, ok := ruleByUID[entry.RuleUID] if !ok { - sch.log.Error("rule not found for instance, ignoring", "rule", entry.RuleDefinitionUID) + sch.log.Error("rule not found for instance, ignoring", "rule", entry.RuleUID) continue } @@ -387,7 +390,7 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { sch.log.Error("error getting cacheId for entry", "msg", err.Error()) } stateForEntry := &state.State{ - AlertRuleUID: entry.RuleDefinitionUID, + AlertRuleUID: entry.RuleUID, OrgID: entry.RuleOrgID, CacheId: cacheId, Labels: lbs, diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 5b1e7cf2707..9a7e22c3561 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -80,7 +80,7 @@ func (st DBstore) DeleteAlertRuleByUID(orgID int64, ruleUID string) error { return err } - _, err = sess.Exec("DELETE FROM alert_instance WHERE def_org_id = ? AND def_uid = ?", orgID, ruleUID) + _, err = sess.Exec("DELETE FROM alert_instance WHERE rule_org_id = ? AND rule_uid = ?", orgID, ruleUID) if err != nil { return err } @@ -109,7 +109,7 @@ func (st DBstore) DeleteNamespaceAlertRules(orgID int64, namespaceUID string) ([ return err } - if _, err := sess.Exec(`DELETE FROM alert_instance WHERE def_org_id = ? AND def_uid NOT IN ( + if _, err := sess.Exec(`DELETE FROM alert_instance WHERE rule_org_id = ? AND rule_uid NOT IN ( SELECT uid FROM alert_rule where org_id = ? )`, orgID, orgID); err != nil { return err @@ -146,7 +146,7 @@ func (st DBstore) DeleteRuleGroupAlertRules(orgID int64, namespaceUID string, ru return err } - if _, err := sess.Exec(`DELETE FROM alert_instance WHERE def_org_id = ? AND def_uid NOT IN ( + if _, err := sess.Exec(`DELETE FROM alert_instance WHERE rule_org_id = ? AND rule_uid NOT IN ( SELECT uid FROM alert_rule where org_id = ? )`, orgID, orgID); err != nil { return err @@ -161,7 +161,7 @@ func (st DBstore) DeleteRuleGroupAlertRules(orgID int64, namespaceUID string, ru // DeleteAlertInstanceByRuleUID is a handler for deleting alert instances by alert rule UID when a rule has been updated func (st DBstore) DeleteAlertInstancesByRuleUID(orgID int64, ruleUID string) error { return st.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - _, err := sess.Exec("DELETE FROM alert_instance WHERE def_org_id = ? AND def_uid = ?", orgID, ruleUID) + _, err := sess.Exec("DELETE FROM alert_instance WHERE rule_org_id = ? AND rule_uid = ?", orgID, ruleUID) if err != nil { return err } diff --git a/pkg/services/ngalert/store/database.go b/pkg/services/ngalert/store/database.go index f5fcf59a4fc..c11dd09455f 100644 --- a/pkg/services/ngalert/store/database.go +++ b/pkg/services/ngalert/store/database.go @@ -14,14 +14,6 @@ var TimeNow = time.Now // AlertDefinitionMaxTitleLength is the maximum length of the alert definition title const AlertDefinitionMaxTitleLength = 190 -// Store is the interface for persisting alert definitions and instances -type Store interface { - GetAlertInstance(*models.GetAlertInstanceQuery) error - ListAlertInstances(*models.ListAlertInstancesQuery) error - SaveAlertInstance(*models.SaveAlertInstanceCommand) error - FetchOrgIds(cmd *models.FetchUniqueOrgIdsQuery) error -} - // AlertingStore is the database interface used by the Alertmanager service. type AlertingStore interface { GetLatestAlertmanagerConfiguration(*models.GetLatestAlertmanagerConfigurationQuery) error diff --git a/pkg/services/ngalert/store/database_mig.go b/pkg/services/ngalert/store/database_mig.go index a8bf9503899..9a77850996c 100644 --- a/pkg/services/ngalert/store/database_mig.go +++ b/pkg/services/ngalert/store/database_mig.go @@ -52,6 +52,8 @@ func AddAlertDefinitionMigrations(mg *migrator.Migrator, defaultIntervalSeconds mg.AddMigration("Add column paused in alert_definition", migrator.NewAddColumnMigration(alertDefinition, &migrator.Column{ Name: "paused", Type: migrator.DB_Bool, Nullable: false, Default: "0", })) + + mg.AddMigration("drop alert_definition table", migrator.NewDropTableMigration("alert_definition")) } // AddAlertDefinitionMigrations should not be modified. @@ -84,6 +86,7 @@ func AddAlertDefinitionVersionMigrations(mg *migrator.Migrator) { mg.AddMigration("alter alert_definition_version table data column to mediumtext in mysql", migrator.NewRawSQLMigration(""). Mysql("ALTER TABLE alert_definition_version MODIFY data MEDIUMTEXT;")) + mg.AddMigration("drop alert_definition_version table", migrator.NewDropTableMigration("alert_definition_version")) } func AlertInstanceMigration(mg *migrator.Migrator) { @@ -112,6 +115,24 @@ func AlertInstanceMigration(mg *migrator.Migrator) { mg.AddMigration("add column current_state_end to alert_instance", migrator.NewAddColumnMigration(alertInstance, &migrator.Column{ Name: "current_state_end", Type: migrator.DB_BigInt, Nullable: false, Default: "0", })) + + mg.AddMigration("remove index def_org_id, def_uid, current_state on alert_instance", migrator.NewDropIndexMigration(alertInstance, alertInstance.Indices[0])) + mg.AddMigration("remove index def_org_id, current_state on alert_instance", migrator.NewDropIndexMigration(alertInstance, alertInstance.Indices[1])) + + mg.AddMigration("rename def_org_id to rule_org_id in alert_instance", migrator.NewRawSQLMigration(""). + Default("ALTER TABLE alert_instance RENAME COLUMN def_org_id TO rule_org_id;"). + Mysql("ALTER TABLE alert_instance CHANGE def_org_id rule_org_id BIGINT;")) + + mg.AddMigration("rename def_uid to rule_uid in alert_instance", migrator.NewRawSQLMigration(""). + Default("ALTER TABLE alert_instance RENAME COLUMN def_uid TO rule_uid;"). + Mysql("ALTER TABLE alert_instance CHANGE def_uid rule_uid VARCHAR(40);")) + + mg.AddMigration("add index rule_org_id, rule_uid, current_state on alert_instance", migrator.NewAddIndexMigration(alertInstance, &migrator.Index{ + Cols: []string{"rule_org_id", "rule_uid", "current_state"}, Type: migrator.IndexType, + })) + mg.AddMigration("add index rule_org_id, current_state on alert_instance", migrator.NewAddIndexMigration(alertInstance, &migrator.Index{ + Cols: []string{"rule_org_id", "current_state"}, Type: migrator.IndexType, + })) } func AddAlertRuleMigrations(mg *migrator.Migrator, defaultIntervalSeconds int64) { diff --git a/pkg/services/ngalert/store/instance_database.go b/pkg/services/ngalert/store/instance_database.go index 9b796fd5bc1..5c4bc0d7b1e 100644 --- a/pkg/services/ngalert/store/instance_database.go +++ b/pkg/services/ngalert/store/instance_database.go @@ -13,7 +13,7 @@ type InstanceStore interface { GetAlertInstance(cmd *models.GetAlertInstanceQuery) error ListAlertInstances(cmd *models.ListAlertInstancesQuery) error SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error - FetchOrgIds(cmd *models.FetchUniqueOrgIdsQuery) error + FetchOrgIds() ([]int64, error) } // GetAlertInstance is a handler for retrieving an alert instance based on OrgId, AlertDefintionID, and @@ -24,8 +24,8 @@ func (st DBstore) GetAlertInstance(cmd *models.GetAlertInstanceQuery) error { s := strings.Builder{} s.WriteString(`SELECT * FROM alert_instance WHERE - def_org_id=? AND - def_uid=? AND + rule_org_id=? AND + rule_uid=? AND labels_hash=? `) @@ -38,7 +38,7 @@ func (st DBstore) GetAlertInstance(cmd *models.GetAlertInstanceQuery) error { has, err := sess.SQL(s.String(), params...).Get(&instance) if !has { - return fmt.Errorf("instance not found for labels %v (hash: %v), alert definition %v (org %v)", cmd.Labels, hash, cmd.RuleUID, cmd.RuleOrgID) + return fmt.Errorf("instance not found for labels %v (hash: %v), alert rule %v (org %v)", cmd.Labels, hash, cmd.RuleUID, cmd.RuleOrgID) } if err != nil { return err @@ -63,10 +63,10 @@ func (st DBstore) ListAlertInstances(cmd *models.ListAlertInstancesQuery) error params = append(params, p...) } - addToQuery("SELECT alert_instance.*, alert_definition.title AS def_title FROM alert_instance LEFT JOIN alert_definition ON alert_instance.def_org_id = alert_definition.org_id AND alert_instance.def_uid = alert_definition.uid WHERE def_org_id = ?", cmd.RuleOrgID) + addToQuery("SELECT alert_instance.*, alert_rule.title AS rule_title FROM alert_instance LEFT JOIN alert_rule ON alert_instance.rule_org_id = alert_rule.org_id AND alert_instance.rule_uid = alert_rule.uid WHERE rule_org_id = ?", cmd.RuleOrgID) if cmd.RuleUID != "" { - addToQuery(` AND def_uid = ?`, cmd.RuleUID) + addToQuery(` AND rule_uid = ?`, cmd.RuleUID) } if cmd.State != "" { @@ -109,8 +109,8 @@ func (st DBstore) SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error upsertSQL := st.SQLStore.Dialect.UpsertSQL( "alert_instance", - []string{"def_org_id", "def_uid", "labels_hash"}, - []string{"def_org_id", "def_uid", "labels", "labels_hash", "current_state", "current_state_since", "current_state_end", "last_eval_time"}) + []string{"rule_org_id", "rule_uid", "labels_hash"}, + []string{"rule_org_id", "rule_uid", "labels", "labels_hash", "current_state", "current_state_since", "current_state_end", "last_eval_time"}) _, err = sess.SQL(upsertSQL, params...).Query() if err != nil { return err @@ -120,10 +120,10 @@ func (st DBstore) SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error }) } -func (st DBstore) FetchOrgIds(cmd *models.FetchUniqueOrgIdsQuery) error { - return st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - orgIds := make([]*models.FetchUniqueOrgIdsQueryResult, 0) +func (st DBstore) FetchOrgIds() ([]int64, error) { + orgIds := []int64{} + err := st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { s := strings.Builder{} params := make([]interface{}, 0) @@ -132,13 +132,13 @@ func (st DBstore) FetchOrgIds(cmd *models.FetchUniqueOrgIdsQuery) error { params = append(params, p...) } - addToQuery("SELECT DISTINCT def_org_id FROM alert_instance") + addToQuery("SELECT DISTINCT rule_org_id FROM alert_instance") if err := sess.SQL(s.String(), params...).Find(&orgIds); err != nil { return err } - - cmd.Result = orgIds return nil }) + + return orgIds, err } diff --git a/pkg/services/ngalert/tests/instance_database_test.go b/pkg/services/ngalert/tests/instance_database_test.go index 79c5532cc5f..95aba4c14a9 100644 --- a/pkg/services/ngalert/tests/instance_database_test.go +++ b/pkg/services/ngalert/tests/instance_database_test.go @@ -172,7 +172,7 @@ func TestAlertInstanceOperations(t *testing.T) { require.Len(t, listQuery.Result, 1) require.Equal(t, saveCmdTwo.RuleOrgID, listQuery.Result[0].RuleOrgID) - require.Equal(t, saveCmdTwo.RuleUID, listQuery.Result[0].RuleDefinitionUID) + require.Equal(t, saveCmdTwo.RuleUID, listQuery.Result[0].RuleUID) require.Equal(t, saveCmdTwo.Labels, listQuery.Result[0].Labels) require.Equal(t, saveCmdTwo.State, listQuery.Result[0].CurrentState) })