public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Storage: Propagate RV to server on update+delete (#111866)

Browse Source
This commit is contained in:
Ryan McKinley
2025-10-01 18:00:02 +03:00
committed by GitHub
parent c0d0121b41
commit c530cacb1c
21 changed files with 183 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/storage/unified/sql/backend.go
+32 -3
View File
@@ -18,6 +18,7 @@ import (
"go.opentelemetry.io/otel/trace/noop"
"google.golang.org/protobuf/proto"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"github.com/grafana/grafana/pkg/util/sqlite"
@@ -405,15 +406,18 @@ func (b *backend) update(ctx context.Context, event resource.WriteEvent) (int64,
// Use rvManager.ExecWithRV instead of direct transaction
rv, err := b.rvManager.ExecWithRV(ctx, event.Key, func(tx db.Tx) (string, error) {
// 1. Update resource
_, err := dbutil.Exec(ctx, tx, sqlResourceUpdate, sqlResourceRequest{
res, err := dbutil.Exec(ctx, tx, sqlResourceUpdate, sqlResourceRequest{
SQLTemplate: sqltemplate.New(b.dialect),
WriteEvent: event,
WriteEvent: event, // includes the RV
Folder: folder,
GUID: event.GUID,
})
if err != nil {
return event.GUID, fmt.Errorf("resource update: %w", err)
}
if err = b.checkConflict(res, event.Key, event.PreviousRV); err != nil {
return event.GUID, err
}
// 2. Insert into resource history
if _, err := dbutil.Exec(ctx, tx, sqlResourceHistoryInsert, sqlResourceRequest{
@@ -460,7 +464,7 @@ func (b *backend) delete(ctx context.Context, event resource.WriteEvent) (int64,
}
rv, err := b.rvManager.ExecWithRV(ctx, event.Key, func(tx db.Tx) (string, error) {
// 1. delete from resource
_, err := dbutil.Exec(ctx, tx, sqlResourceDelete, sqlResourceRequest{
res, err := dbutil.Exec(ctx, tx, sqlResourceDelete, sqlResourceRequest{
SQLTemplate: sqltemplate.New(b.dialect),
WriteEvent: event,
GUID: event.GUID,
@@ -468,6 +472,9 @@ func (b *backend) delete(ctx context.Context, event resource.WriteEvent) (int64,
if err != nil {
return event.GUID, fmt.Errorf("delete resource: %w", err)
}
if err = b.checkConflict(res, event.Key, event.PreviousRV); err != nil {
return event.GUID, err
}
// 2. Add event to resource history
if _, err := dbutil.Exec(ctx, tx, sqlResourceHistoryInsert, sqlResourceRequest{
@@ -504,6 +511,28 @@ func (b *backend) delete(ctx context.Context, event resource.WriteEvent) (int64,
return rv, nil
}
func (b *backend) checkConflict(res db.Result, key *resourcepb.ResourceKey, rv int64) error {
if rv == 0 {
return nil
}
// The RV is part of the update request, and it may no longer be the most recent
rows, err := res.RowsAffected()
if err != nil {
return fmt.Errorf("unable to verify RV: %w", err)
}
if rows == 1 {
return nil // expected one result
}
if rows > 0 {
return fmt.Errorf("multiple rows effected (%d)", rows)
}
return apierrors.NewConflict(schema.GroupResource{
Group: key.Group,
Resource: key.Resource,
}, key.Name, fmt.Errorf("resource version does not match current value"))
}
func (b *backend) ReadResource(ctx context.Context, req *resourcepb.ReadRequest) *resource.BackendReadResponse {
_, span := b.tracer.Start(ctx, tracePrefix+".Read")
defer span.End()
pkg/storage/unified/sql/backend_test.go
-1
View File
@@ -8,7 +8,6 @@ import (
"testing"
"github.com/DATA-DOG/go-sqlmock"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
pkg/storage/unified/sql/data/resource_delete.sql
+1
View File
@@ -5,5 +5,6 @@ DELETE FROM {{ .Ident "resource" }}
AND {{ .Ident "resource" }} = {{ .Arg .WriteEvent.Key.Resource }}
{{ if .WriteEvent.Key.Name }}
AND {{ .Ident "name" }} = {{ .Arg .WriteEvent.Key.Name }}
AND {{ .Ident "resource_version" }} = {{ .Arg .WriteEvent.PreviousRV }}
{{ end }}
;
pkg/storage/unified/sql/data/resource_update.sql
+1
View File
@@ -10,4 +10,5 @@ UPDATE {{ .Ident "resource" }}
AND {{ .Ident "resource" }} = {{ .Arg .WriteEvent.Key.Resource }}
AND {{ .Ident "namespace" }} = {{ .Arg .WriteEvent.Key.Namespace }}
AND {{ .Ident "name" }} = {{ .Arg .WriteEvent.Key.Name }}
AND {{ .Ident "resource_version" }} = {{ .Arg .WriteEvent.PreviousRV }}
;
pkg/storage/unified/sql/list_iterator_test.go
+13 -5
View File
@@ -87,14 +87,23 @@ func TestIntegrationListIter(t *testing.T) {
Group: item.group,
Name: item.name,
},
Value: item.value,
PreviousRV: 0,
Value: item.value,
},
})
if err != nil {
return fmt.Errorf("failed to insert test data: %w", err)
}
_, err = dbutil.Exec(ctx, tx, sqlResourceUpdate, sqlResourceRequest{
if _, err = dbutil.Exec(ctx, tx, sqlResourceUpdateRV, sqlResourceUpdateRVRequest{
SQLTemplate: sqltemplate.New(dialect),
GUIDToRV: map[string]int64{
item.guid: item.resourceVersion,
},
}); err != nil {
return fmt.Errorf("failed to insert test data: %w", err)
}
if _, err = dbutil.Exec(ctx, tx, sqlResourceUpdate, sqlResourceRequest{
SQLTemplate: sqltemplate.New(dialect),
GUID: item.guid,
ResourceVersion: item.resourceVersion,
@@ -110,8 +119,7 @@ func TestIntegrationListIter(t *testing.T) {
PreviousRV: item.resourceVersion,
Type: 1,
},
})
if err != nil {
}); err != nil {
return fmt.Errorf("failed to insert resource version: %w", err)
}
}
pkg/storage/unified/sql/queries_test.go
+16
View File
@@ -31,6 +31,21 @@ func TestUnifiedStorageQueries(t *testing.T) {
},
},
},
{
Name: "with rv",
Data: &sqlResourceRequest{
SQLTemplate: mocks.NewTestingSQLTemplate(),
WriteEvent: resource.WriteEvent{
Key: &resourcepb.ResourceKey{
Namespace: "nn",
Group: "gg",
Resource: "rr",
Name: "name",
},
PreviousRV: 1234,
},
},
},
},
sqlResourceInsert: {
{
@@ -63,6 +78,7 @@ func TestUnifiedStorageQueries(t *testing.T) {
Resource: "rr",
Name: "name",
},
PreviousRV: 1759304090100678,
},
Folder: "fldr",
},
pkg/storage/unified/sql/rv_manager.go
+1 -1
View File
@@ -263,7 +263,7 @@ func (m *resourceVersionManager) execBatch(ctx context.Context, group, resource
attribute.Int("operation_index", i),
attribute.String("error", err.Error()),
))
return fmt.Errorf("failed to execute function: %w", err)
return err
}
guids[i] = guid
}
pkg/storage/unified/sql/testdata/mysql--resource_delete-simple.sql
Vendored
+1
View File
@@ -4,4 +4,5 @@ DELETE FROM `resource`
AND `group` = 'gg'
AND `resource` = 'rr'
AND `name` = 'name'
AND `resource_version` = 0
;
pkg/storage/unified/sql/testdata/mysql--resource_delete-with rv.sql
Vendored Executable
+8
View File
@@ -0,0 +1,8 @@
DELETE FROM `resource`
WHERE 1 = 1
AND `namespace` = 'nn'
AND `group` = 'gg'
AND `resource` = 'rr'
AND `name` = 'name'
AND `resource_version` = 1234
;
pkg/storage/unified/sql/testdata/mysql--resource_update-single path.sql
Vendored
+1
View File
@@ -10,4 +10,5 @@ UPDATE `resource`
AND `resource` = 'rr'
AND `namespace` = 'nn'
AND `name` = 'name'
AND `resource_version` = 1759304090100678
;
pkg/storage/unified/sql/testdata/postgres--resource_delete-simple.sql
Vendored
+1
View File
@@ -4,4 +4,5 @@ DELETE FROM "resource"
AND "group" = 'gg'
AND "resource" = 'rr'
AND "name" = 'name'
AND "resource_version" = 0
;
pkg/storage/unified/sql/testdata/postgres--resource_delete-with rv.sql
Vendored Executable
+8
View File
@@ -0,0 +1,8 @@
DELETE FROM "resource"
WHERE 1 = 1
AND "namespace" = 'nn'
AND "group" = 'gg'
AND "resource" = 'rr'
AND "name" = 'name'
AND "resource_version" = 1234
;
pkg/storage/unified/sql/testdata/postgres--resource_update-single path.sql
Vendored
+1
View File
@@ -10,4 +10,5 @@ UPDATE "resource"
AND "resource" = 'rr'
AND "namespace" = 'nn'
AND "name" = 'name'
AND "resource_version" = 1759304090100678
;
pkg/storage/unified/sql/testdata/sqlite--resource_delete-simple.sql
Vendored
+1
View File
@@ -4,4 +4,5 @@ DELETE FROM "resource"
AND "group" = 'gg'
AND "resource" = 'rr'
AND "name" = 'name'
AND "resource_version" = 0
;
pkg/storage/unified/sql/testdata/sqlite--resource_delete-with rv.sql
Vendored Executable
+8
View File
@@ -0,0 +1,8 @@
DELETE FROM "resource"
WHERE 1 = 1
AND "namespace" = 'nn'
AND "group" = 'gg'
AND "resource" = 'rr'
AND "name" = 'name'
AND "resource_version" = 1234
;
pkg/storage/unified/sql/testdata/sqlite--resource_update-single path.sql
Vendored
+1
View File
@@ -10,4 +10,5 @@ UPDATE "resource"
AND "resource" = 'rr'
AND "namespace" = 'nn'
AND "name" = 'name'
AND "resource_version" = 1759304090100678
;