From 1e8f1f74ea87dc3e0d8391d7db761aa29e624183 Mon Sep 17 00:00:00 2001 From: Renato Costa <103441181+renatolabs@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:51:15 -0500 Subject: [PATCH] unified-storage: apply backwards compatibility changes outside sqlkv (#115954) --- .../data/sqlkv_insert_legacy_resource.sql | 12 +- .../sqlkv_insert_legacy_resource_history.sql | 27 +--- .../data/sqlkv_update_legacy_resource.sql | 6 +- .../sqlkv_update_legacy_resource_history.sql | 5 + pkg/storage/unified/resource/datastore.go | 127 +++++++++++++++++- pkg/storage/unified/resource/sqlkv.go | 83 ++---------- .../unified/resource/storage_backend.go | 7 +- 7 files changed, 148 insertions(+), 119 deletions(-) create mode 100644 pkg/storage/unified/resource/data/sqlkv_update_legacy_resource_history.sql diff --git a/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource.sql b/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource.sql index 1f58bd28b43..2c034f4d757 100644 --- a/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource.sql +++ b/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource.sql @@ -11,7 +11,7 @@ INSERT INTO {{ .Ident "resource" }} {{ .Ident "previous_resource_version" }} ) VALUES ( - COALESCE({{ .Arg .Value }}, ""), + (SELECT {{ .Ident "value" }} FROM {{ .Ident "resource_history" }} WHERE {{ .Ident "guid" }} = {{ .Arg .GUID }}), {{ .Arg .GUID }}, {{ .Arg .Group }}, {{ .Arg .Resource }}, @@ -19,13 +19,5 @@ VALUES ( {{ .Arg .Name }}, {{ .Arg .Action }}, {{ .Arg .Folder }}, - CASE WHEN {{ .Arg .Action }} = 1 THEN 0 ELSE ( - SELECT {{ .Ident "resource_version" }} - FROM {{ .Ident "resource" }} - WHERE {{ .Ident "group" }} = {{ .Arg .Group }} - AND {{ .Ident "resource" }} = {{ .Arg .Resource }} - AND {{ .Ident "namespace" }} = {{ .Arg .Namespace }} - AND {{ .Ident "name" }} = {{ .Arg .Name }} - ORDER BY {{ .Ident "resource_version" }} DESC LIMIT 1 - ) END + {{ .Arg .PreviousRV }} ); diff --git a/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource_history.sql b/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource_history.sql index d52aac5063d..437d3ae9107 100644 --- a/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource_history.sql +++ b/pkg/storage/unified/resource/data/sqlkv_insert_legacy_resource_history.sql @@ -7,9 +7,7 @@ INSERT INTO {{ .Ident "resource_history" }} {{ .Ident "namespace" }}, {{ .Ident "name" }}, {{ .Ident "action" }}, - {{ .Ident "folder" }}, - {{ .Ident "previous_resource_version" }}, - {{ .Ident "generation" }} + {{ .Ident "folder" }} ) VALUES ( COALESCE({{ .Arg .Value }}, ""), @@ -19,26 +17,5 @@ VALUES ( {{ .Arg .Namespace }}, {{ .Arg .Name }}, {{ .Arg .Action }}, - {{ .Arg .Folder }}, - CASE WHEN {{ .Arg .Action }} = 1 THEN 0 ELSE ( - SELECT {{ .Ident "resource_version" }} - FROM {{ .Ident "resource_history" }} - WHERE {{ .Ident "group" }} = {{ .Arg .Group }} - AND {{ .Ident "resource" }} = {{ .Arg .Resource }} - AND {{ .Ident "namespace" }} = {{ .Arg .Namespace }} - AND {{ .Ident "name" }} = {{ .Arg .Name }} - ORDER BY {{ .Ident "resource_version" }} DESC LIMIT 1 - ) END, - CASE - WHEN {{ .Arg .Action }} = 1 THEN 1 - WHEN {{ .Arg .Action }} = 3 THEN 0 - ELSE 1 + ( - SELECT COUNT(1) - FROM {{ .Ident "resource_history" }} - WHERE {{ .Ident "group" }} = {{ .Arg .Group }} - AND {{ .Ident "resource" }} = {{ .Arg .Resource }} - AND {{ .Ident "namespace" }} = {{ .Arg .Namespace }} - AND {{ .Ident "name" }} = {{ .Arg .Name }} - ) - END + {{ .Arg .Folder }} ); diff --git a/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource.sql b/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource.sql index 1565d0894a4..3c9f92d4de5 100644 --- a/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource.sql +++ b/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource.sql @@ -1,8 +1,10 @@ UPDATE {{ .Ident "resource" }} SET - {{ .Ident "value" }} = {{ .Arg .Value }}, + {{ .Ident "guid" }} = {{ .Arg .GUID }}, + {{ .Ident "value" }} = (SELECT {{ .Ident "value" }} FROM {{ .Ident "resource_history" }} WHERE {{ .Ident "guid" }} = {{ .Arg .GUID }}), {{ .Ident "action" }} = {{ .Arg .Action }}, - {{ .Ident "folder" }} = {{ .Arg .Folder }} + {{ .Ident "folder" }} = {{ .Arg .Folder }}, + {{ .Ident "previous_resource_version" }} = {{ .Arg .PreviousRV }} WHERE {{ .Ident "group" }} = {{ .Arg .Group }} AND {{ .Ident "resource" }} = {{ .Arg .Resource }} AND {{ .Ident "namespace" }} = {{ .Arg .Namespace }} diff --git a/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource_history.sql b/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource_history.sql new file mode 100644 index 00000000000..ac1f135fd76 --- /dev/null +++ b/pkg/storage/unified/resource/data/sqlkv_update_legacy_resource_history.sql @@ -0,0 +1,5 @@ +UPDATE {{ .Ident "resource_history" }} +SET + {{ .Ident "previous_resource_version" }} = {{ .Arg .PreviousRV }}, + {{ .Ident "generation" }} = {{ .Arg .Generation }} + WHERE {{ .Ident "guid" }} = {{ .Arg .GUID }}; diff --git a/pkg/storage/unified/resource/datastore.go b/pkg/storage/unified/resource/datastore.go index 7a1b614323e..313f7d43852 100644 --- a/pkg/storage/unified/resource/datastore.go +++ b/pkg/storage/unified/resource/datastore.go @@ -12,6 +12,9 @@ import ( "time" "github.com/grafana/grafana/pkg/apimachinery/validation" + "github.com/grafana/grafana/pkg/storage/unified/sql/db" + "github.com/grafana/grafana/pkg/storage/unified/sql/dbutil" + "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" gocache "github.com/patrickmn/go-cache" ) @@ -306,10 +309,6 @@ func (d *dataStore) GetResourceKeyAtRevision(ctx context.Context, key GetRequest return DataKey{}, fmt.Errorf("invalid get request key: %w", err) } - if rv == 0 { - rv = math.MaxInt64 - } - listKey := ListRequestKey(key) iter := d.ListResourceKeysAtRevision(ctx, ListRequestOptions{Key: listKey, ResourceVersion: rv}) @@ -598,7 +597,7 @@ func ParseKey(key string) (DataKey, error) { }, nil } -// Temporary while we need to support unified/sql/backend compatibility +// Temporary while we need to support unified/sql/backend compatibility. // Remove once we stop using RvManager in storage_backend.go func ParseKeyWithGUID(key string) (DataKey, error) { parts := strings.Split(key, "/") @@ -815,3 +814,121 @@ func (d *dataStore) getGroupResources(ctx context.Context) ([]GroupResource, err return results, nil } + +// TODO: remove when backwards compatibility is no longer needed. +var ( + sqlKVUpdateLegacyResourceHistory = mustTemplate("sqlkv_update_legacy_resource_history.sql") + sqlKVInsertLegacyResource = mustTemplate("sqlkv_insert_legacy_resource.sql") + sqlKVUpdateLegacyResource = mustTemplate("sqlkv_update_legacy_resource.sql") +) + +// TODO: remove when backwards compatibility is no longer needed. +type sqlKVLegacySaveRequest struct { + sqltemplate.SQLTemplate + GUID string + Group string + Resource string + Namespace string + Name string + Action int64 + Folder string + PreviousRV int64 +} + +func (req sqlKVLegacySaveRequest) Validate() error { + return nil +} + +// TODO: remove when backwards compatibility is no longer needed. +type sqlKVLegacyUpdateHistoryRequest struct { + sqltemplate.SQLTemplate + GUID string + PreviousRV int64 + Generation int64 +} + +func (req sqlKVLegacyUpdateHistoryRequest) Validate() error { + return nil +} + +// applyBackwardsCompatibleChanges updates the `resource` and `resource_history` tables +// to make sure the sqlkv implementation is backwards-compatible with the existing sql backend. +// Specifically, it will update the `resource_history` table to include the previous resource version +// and generation, which come from the `WriteEvent`, and also make the corresponding change on the +// `resource` table, no longer used in the storage backend. +// +// TODO: remove when backwards compatibility is no longer needed. +func (d *dataStore) applyBackwardsCompatibleChanges(ctx context.Context, tx db.Tx, event WriteEvent, key DataKey) error { + kv, isSQLKV := d.kv.(*sqlKV) + if !isSQLKV { + return nil + } + + _, err := dbutil.Exec(ctx, tx, sqlKVUpdateLegacyResourceHistory, sqlKVLegacyUpdateHistoryRequest{ + SQLTemplate: sqltemplate.New(kv.dialect), + GUID: key.GUID, + PreviousRV: event.PreviousRV, + Generation: event.Object.GetGeneration(), + }) + + if err != nil { + return fmt.Errorf("compatibility layer: failed to insert to resource: %w", err) + } + + var action int64 + switch key.Action { + case DataActionCreated: + action = 1 + case DataActionUpdated: + action = 2 + case DataActionDeleted: + action = 3 + } + + switch key.Action { + case DataActionCreated: + _, err := dbutil.Exec(ctx, tx, sqlKVInsertLegacyResource, sqlKVLegacySaveRequest{ + SQLTemplate: sqltemplate.New(kv.dialect), + GUID: key.GUID, + Group: key.Group, + Resource: key.Resource, + Namespace: key.Namespace, + Name: key.Name, + Action: action, + Folder: key.Folder, + PreviousRV: event.PreviousRV, + }) + + if err != nil { + return fmt.Errorf("compatibility layer: failed to insert to resource: %w", err) + } + case DataActionUpdated: + _, err := dbutil.Exec(ctx, tx, sqlKVUpdateLegacyResource, sqlKVLegacySaveRequest{ + SQLTemplate: sqltemplate.New(kv.dialect), + GUID: key.GUID, + Group: key.Group, + Resource: key.Resource, + Namespace: key.Namespace, + Name: key.Name, + Folder: key.Folder, + PreviousRV: event.PreviousRV, + }) + + if err != nil { + return fmt.Errorf("compatibility layer: failed to update resource: %w", err) + } + case DataActionDeleted: + _, err := dbutil.Exec(ctx, tx, sqlKVDeleteLegacyResource, sqlKVLegacySaveRequest{ + SQLTemplate: sqltemplate.New(kv.dialect), + Resource: key.Resource, + Namespace: key.Namespace, + Name: key.Name, + }) + + if err != nil { + return fmt.Errorf("compatibility layer: failed to delete from resource: %w", err) + } + } + + return nil +} diff --git a/pkg/storage/unified/resource/sqlkv.go b/pkg/storage/unified/resource/sqlkv.go index 6d406294a96..bae3c776d79 100644 --- a/pkg/storage/unified/resource/sqlkv.go +++ b/pkg/storage/unified/resource/sqlkv.go @@ -44,8 +44,6 @@ var ( sqlKVInsertData = mustTemplate("sqlkv_insert_datastore.sql") sqlKVUpdateData = mustTemplate("sqlkv_update_datastore.sql") sqlKVInsertLegacyResourceHistory = mustTemplate("sqlkv_insert_legacy_resource_history.sql") - sqlKVInsertLegacyResource = mustTemplate("sqlkv_insert_legacy_resource.sql") - sqlKVUpdateLegacyResource = mustTemplate("sqlkv_update_legacy_resource.sql") sqlKVDeleteLegacyResource = mustTemplate("sqlkv_delete_legacy_resource.sql") sqlKVDelete = mustTemplate("sqlkv_delete.sql") sqlKVBatchDelete = mustTemplate("sqlkv_batch_delete.sql") @@ -157,26 +155,6 @@ func (req sqlKVSaveRequest) Validate() error { return req.sqlKVSectionKey.Validate() } -type sqlKVLegacySaveRequest struct { - sqltemplate.SQLTemplate - Value []byte - GUID string - Group string - Resource string - Namespace string - Name string - Action int64 - Folder string -} - -func (req sqlKVLegacySaveRequest) Validate() error { - return nil -} - -func (req sqlKVLegacySaveRequest) Results() ([]byte, error) { - return req.Value, nil -} - type sqlKVKeysRequest struct { sqltemplate.SQLTemplate sqlKVSection @@ -392,7 +370,7 @@ func (w *sqlWriteCloser) Close() error { // used to keep backwards compatibility between sql-based kvstore and unified/sql/backend tx, ok := rvmanager.TxFromCtx(w.ctx) if !ok { - // temporary save for dataStore without rvmanager + // temporary save for dataStore without rvmanager (non backwards-compatible) // we can use the same template as the event one after we: // - move PK from GUID to key_path // - remove all unnecessary columns (or at least their NOT NULL constraints) @@ -429,11 +407,12 @@ func (w *sqlWriteCloser) Close() error { return nil } - // special, temporary save that includes all the fields in resource_history that are not relevant for the kvstore, - // as well as the resource table. This is only called if an RvManager was passed to storage_backend, as that - // component will be responsible for populating the resource_version and key_path columns - // note that we are not touching resource_version table, neither the resource_version columns or the key_path column - // as the RvManager will be responsible for this + // special, temporary backwards-compatible save that includes all the fields in resource_history that are not relevant + // for the kvstore, as well as the resource table. This is only called if an RvManager was passed to storage_backend, as that + // component will be responsible for populating the resource_version and key_path columns. + // For full backwards-compatibility, the `Save` function needs to be called within a callback that updates the resource_history + // table with `previous_resource_version` and `generation` and updates the `resource` table accordingly. See the + // storage_backend for the full implementation. dataKey, err := ParseKeyWithGUID(w.sectionKey.Key) if err != nil { return fmt.Errorf("failed to parse key: %w", err) @@ -448,7 +427,7 @@ func (w *sqlWriteCloser) Close() error { case DataActionDeleted: action = 3 default: - return fmt.Errorf("failed to parse key: %w", err) + return fmt.Errorf("failed to parse key: invalid action") } _, err = dbutil.Exec(w.ctx, tx, sqlKVInsertLegacyResourceHistory, sqlKVSaveRequest{ @@ -468,52 +447,6 @@ func (w *sqlWriteCloser) Close() error { return fmt.Errorf("failed to save to resource_history: %w", err) } - switch dataKey.Action { - case DataActionCreated: - _, err = dbutil.Exec(w.ctx, tx, sqlKVInsertLegacyResource, sqlKVLegacySaveRequest{ - SQLTemplate: sqltemplate.New(w.kv.dialect), - Value: w.buf.Bytes(), - GUID: dataKey.GUID, - Group: dataKey.Group, - Resource: dataKey.Resource, - Namespace: dataKey.Namespace, - Name: dataKey.Name, - Action: action, - Folder: dataKey.Folder, - }) - - if err != nil { - return fmt.Errorf("failed to insert to resource: %w", err) - } - case DataActionUpdated: - _, err = dbutil.Exec(w.ctx, tx, sqlKVUpdateLegacyResource, sqlKVLegacySaveRequest{ - SQLTemplate: sqltemplate.New(w.kv.dialect), - Value: w.buf.Bytes(), - Group: dataKey.Group, - Resource: dataKey.Resource, - Namespace: dataKey.Namespace, - Name: dataKey.Name, - Action: action, - Folder: dataKey.Folder, - }) - - if err != nil { - return fmt.Errorf("failed to update resource: %w", err) - } - case DataActionDeleted: - _, err = dbutil.Exec(w.ctx, tx, sqlKVDeleteLegacyResource, sqlKVLegacySaveRequest{ - SQLTemplate: sqltemplate.New(w.kv.dialect), - Group: dataKey.Group, - Resource: dataKey.Resource, - Namespace: dataKey.Namespace, - Name: dataKey.Name, - }) - - if err != nil { - return fmt.Errorf("failed to delete from resource: %w", err) - } - } - return nil } diff --git a/pkg/storage/unified/resource/storage_backend.go b/pkg/storage/unified/resource/storage_backend.go index 13f2b9d6159..dffecbd789c 100644 --- a/pkg/storage/unified/resource/storage_backend.go +++ b/pkg/storage/unified/resource/storage_backend.go @@ -332,11 +332,14 @@ func (k *kvStorageBackend) WriteEvent(ctx context.Context, event WriteEvent) (in dataKey.GUID = uuid.New().String() var err error rv, err = k.rvManager.ExecWithRV(ctx, event.Key, func(tx db.Tx) (string, error) { - err := k.dataStore.Save(rvmanager.ContextWithTx(ctx, tx), dataKey, bytes.NewReader(event.Value)) - if err != nil { + if err := k.dataStore.Save(rvmanager.ContextWithTx(ctx, tx), dataKey, bytes.NewReader(event.Value)); err != nil { return "", fmt.Errorf("failed to write data: %w", err) } + if err := k.dataStore.applyBackwardsCompatibleChanges(ctx, tx, event, dataKey); err != nil { + return "", fmt.Errorf("failed to apply backwards compatible updates: %w", err) + } + return dataKey.GUID, nil }) if err != nil {