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

@@ -7,8 +7,8 @@ import (
 
 // AlertInstance represents a single alert instance.
 type AlertInstance struct {
-	RuleOrgID         int64  `xorm:"def_org_id"`
+	RuleOrgID         int64  `xorm:"rule_org_id"`
-	RuleUID           string `xorm:"def_uid"`
+	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"`
+	RuleOrgID         int64             `xorm:"rule_org_id" json:"ruleOrgId"`
-	RuleDefinitionUID string            `xorm:"def_uid" json:"definitionUid"`
+	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 {
+// ValidateAlertInstance validates that the alert instance contains an alert rule id,
-	DefinitionOrgID int64 `xorm:"def_org_id" json:"definitionOrgId"`
-}
-
-// ValidateAlertInstance validates that the alert instance contains an alert definition 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() {

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()
+			waitErr := dispatcherGroup.Wait()
-			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())
 			}
-			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{}
+	orgIds, err := sch.instanceStore.FetchOrgIds()
-	if err := sch.instanceStore.FetchOrgIds(&orgIdsCmd); err != nil {
+	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,

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
 		}

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

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) {

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
+				rule_org_id=? AND
-				def_uid=? 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{"rule_org_id", "rule_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", "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 {
+func (st DBstore) FetchOrgIds() ([]int64, error) {
-	return st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
+	orgIds := []int64{}
-		orgIds := make([]*models.FetchUniqueOrgIdsQueryResult, 0)
 
+	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
 }

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)
 	})