Alerting: Create abstraction for launching transactions and refactor existing transaction management to use it (#46216)

* Remove InTransaction from RuleStore and make it its own interface

* Ensure that ctx-based is clear from name

* Resolve merge conflicts

* Refactor tests to work in terms of the introduced abstraction rather than concrete dbstore
This commit is contained in:
Alexander Weaver 2022-03-15 11:48:42 -05:00 committed by GitHub
parent ecdbcd4941
commit 92716cb602
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 72 additions and 18 deletions

View File

@ -63,6 +63,7 @@ type API struct {
ExpressionService *expr.Service
QuotaService *quota.QuotaService
Schedule schedule.ScheduleService
TransactionManager store.TransactionManager
RuleStore store.RuleStore
InstanceStore store.InstanceStore
AlertingStore AlertingStore
@ -97,7 +98,13 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) {
api.RegisterRulerApiEndpoints(NewForkedRuler(
api.DatasourceCache,
NewLotexRuler(proxy, logger),
&RulerSrv{DatasourceCache: api.DatasourceCache, QuotaService: api.QuotaService, scheduleService: api.Schedule, store: api.RuleStore, log: logger, cfg: &api.Cfg.UnifiedAlerting},
&RulerSrv{
DatasourceCache: api.DatasourceCache,
QuotaService: api.QuotaService,
scheduleService: api.Schedule,
store: api.RuleStore,
xactManager: api.TransactionManager,
log: logger, cfg: &api.Cfg.UnifiedAlerting},
), m)
api.RegisterTestingApiEndpoints(NewForkedTestingApi(
&TestingApiSrv{

View File

@ -27,6 +27,7 @@ import (
)
type RulerSrv struct {
xactManager store.TransactionManager
store store.RuleStore
DatasourceCache datasources.CacheService
QuotaService *quota.QuotaService
@ -263,7 +264,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, namespace *mod
// TODO add create rules authz logic
var groupChanges *changes = nil
err := srv.store.InTransaction(c.Req.Context(), func(tranCtx context.Context) error {
err := srv.xactManager.InTransaction(c.Req.Context(), func(tranCtx context.Context) error {
var err error
groupChanges, err = calculateChanges(tranCtx, srv.store, c.SignedInUser.OrgId, namespace, groupName, rules)
if err != nil {

View File

@ -144,6 +144,7 @@ func (ng *AlertNG) init() error {
DataProxy: ng.DataProxy,
QuotaService: ng.QuotaService,
SecretsService: ng.SecretsService,
TransactionManager: store,
InstanceStore: store,
RuleStore: store,
AlertingStore: store,

View File

@ -48,7 +48,6 @@ type RuleStore interface {
GetNamespaceByTitle(context.Context, string, int64, *models.SignedInUser, bool) (*models.Folder, error)
GetOrgRuleGroups(ctx context.Context, query *ngmodels.ListOrgRuleGroupsQuery) error
UpsertAlertRules(ctx context.Context, rule []UpsertRule) error
InTransaction(ctx context.Context, f func(ctx context.Context) error) error
}
func getAlertRuleByUID(sess *sqlstore.DBSession, alertRuleUID string, orgID int64) (*ngmodels.AlertRule, error) {
@ -502,7 +501,3 @@ WHERE org_id = ?`
return nil
})
}
func (st *DBstore) InTransaction(ctx context.Context, f func(ctx context.Context) error) error {
return st.SQLStore.InTransaction(ctx, f)
}

View File

@ -2,9 +2,12 @@ package store_test
import (
"context"
"fmt"
"testing"
"github.com/grafana/grafana/pkg/services/ngalert"
"github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/ngalert/tests"
"github.com/stretchr/testify/require"
)
@ -12,14 +15,14 @@ import (
const testAlertingIntervalSeconds = 10
func TestProvisioningStore(t *testing.T) {
_, dbstore := tests.SetupTestEnv(t, testAlertingIntervalSeconds)
store, xact := createSut(tests.SetupTestEnv(t, testAlertingIntervalSeconds))
t.Run("Default provenance of a known type is None", func(t *testing.T) {
rule := models.AlertRule{
UID: "asdf",
}
provenance, err := dbstore.GetProvenance(context.Background(), &rule)
provenance, err := store.GetProvenance(context.Background(), &rule)
require.NoError(t, err)
require.Equal(t, models.ProvenanceNone, provenance)
@ -29,10 +32,10 @@ func TestProvisioningStore(t *testing.T) {
rule := models.AlertRule{
UID: "123",
}
err := dbstore.SetProvenance(context.Background(), &rule, models.ProvenanceFile)
err := store.SetProvenance(context.Background(), &rule, models.ProvenanceFile)
require.NoError(t, err)
p, err := dbstore.GetProvenance(context.Background(), &rule)
p, err := store.GetProvenance(context.Background(), &rule)
require.NoError(t, err)
require.Equal(t, models.ProvenanceFile, p)
@ -47,10 +50,10 @@ func TestProvisioningStore(t *testing.T) {
UID: "456",
OrgID: 3,
}
err := dbstore.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceFile)
err := store.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceFile)
require.NoError(t, err)
p, err := dbstore.GetProvenance(context.Background(), &ruleOrg3)
p, err := store.GetProvenance(context.Background(), &ruleOrg3)
require.NoError(t, err)
require.Equal(t, models.ProvenanceNone, p)
@ -65,19 +68,54 @@ func TestProvisioningStore(t *testing.T) {
UID: "789",
OrgID: 3,
}
err := dbstore.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceFile)
err := store.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceFile)
require.NoError(t, err)
err = dbstore.SetProvenance(context.Background(), &ruleOrg3, models.ProvenanceFile)
err = store.SetProvenance(context.Background(), &ruleOrg3, models.ProvenanceFile)
require.NoError(t, err)
err = dbstore.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceApi)
err = store.SetProvenance(context.Background(), &ruleOrg2, models.ProvenanceApi)
require.NoError(t, err)
p, err := dbstore.GetProvenance(context.Background(), &ruleOrg2)
p, err := store.GetProvenance(context.Background(), &ruleOrg2)
require.NoError(t, err)
require.Equal(t, models.ProvenanceApi, p)
p, err = dbstore.GetProvenance(context.Background(), &ruleOrg3)
p, err = store.GetProvenance(context.Background(), &ruleOrg3)
require.NoError(t, err)
require.Equal(t, models.ProvenanceFile, p)
})
t.Run("Store saves provenance type when contextual transaction is applied", func(t *testing.T) {
rule := models.AlertRule{
UID: "456",
}
err := xact.InTransaction(context.Background(), func(ctx context.Context) error {
return store.SetProvenance(ctx, &rule, models.ProvenanceFile)
})
require.NoError(t, err)
provenance, err := store.GetProvenance(context.Background(), &rule)
require.NoError(t, err)
require.Equal(t, models.ProvenanceFile, provenance)
})
t.Run("Contextual transaction which errors before saving rolls back type update", func(t *testing.T) {
rule := models.AlertRule{
UID: "789",
}
_ = xact.InTransaction(context.Background(), func(ctx context.Context) error {
err := store.SetProvenance(ctx, &rule, models.ProvenanceFile)
require.NoError(t, err)
return fmt.Errorf("something happened!")
})
provenance, err := store.GetProvenance(context.Background(), &rule)
require.NoError(t, err)
require.Equal(t, models.ProvenanceNone, provenance)
})
}
func createSut(_ *ngalert.AlertNG, db *store.DBstore) (store.ProvisioningStore, store.TransactionManager) {
return db, db
}

View File

@ -0,0 +1,12 @@
package store
import "context"
// TransactionManager represents the ability to issue and close transactions through contexts.
type TransactionManager interface {
InTransaction(ctx context.Context, work func(ctx context.Context) error) error
}
func (st *DBstore) InTransaction(ctx context.Context, f func(ctx context.Context) error) error {
return st.SQLStore.InTransaction(ctx, f)
}