From 9c49c601f197a4977b7410e9944a4abe73c93144 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 1 Apr 2025 20:38:23 +0300 Subject: [PATCH] Unistore: Add generation column (#102222) --- pkg/storage/unified/sql/backend.go | 5 +++- .../sql/data/resource_history_insert.sql | 2 ++ .../sql/data/resource_history_prune.sql | 18 ++++++++----- .../unified/sql/db/migrations/resource_mig.go | 14 +++++++++- pkg/storage/unified/sql/queries.go | 6 +++-- pkg/storage/unified/sql/queries_test.go | 27 ++++++++++++++----- ...ry_insert-insert into resource_history.sql | 2 ++ ...rce_history_prune-collapse-generations.sql | 23 ++++++++++++++++ ...l--resource_history_prune-max-versions.sql | 21 +++++++++++++++ .../mysql--resource_history_prune-simple.sql | 23 ---------------- ...ry_insert-insert into resource_history.sql | 2 ++ ...rce_history_prune-collapse-generations.sql | 23 ++++++++++++++++ ...s--resource_history_prune-max-versions.sql | 21 +++++++++++++++ ...ostgres--resource_history_prune-simple.sql | 23 ---------------- ...ry_insert-insert into resource_history.sql | 2 ++ ...rce_history_prune-collapse-generations.sql | 23 ++++++++++++++++ ...e--resource_history_prune-max-versions.sql | 21 +++++++++++++++ .../sqlite--resource_history_prune-simple.sql | 23 ---------------- .../api/dashboards/api_dashboards_test.go | 2 +- 19 files changed, 194 insertions(+), 87 deletions(-) create mode 100755 pkg/storage/unified/sql/testdata/mysql--resource_history_prune-collapse-generations.sql create mode 100755 pkg/storage/unified/sql/testdata/mysql--resource_history_prune-max-versions.sql delete mode 100755 pkg/storage/unified/sql/testdata/mysql--resource_history_prune-simple.sql create mode 100755 pkg/storage/unified/sql/testdata/postgres--resource_history_prune-collapse-generations.sql create mode 100755 pkg/storage/unified/sql/testdata/postgres--resource_history_prune-max-versions.sql delete mode 100755 pkg/storage/unified/sql/testdata/postgres--resource_history_prune-simple.sql create mode 100755 pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-collapse-generations.sql create mode 100755 pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-max-versions.sql delete mode 100755 pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-simple.sql diff --git a/pkg/storage/unified/sql/backend.go b/pkg/storage/unified/sql/backend.go index f69f2166789..74400a5858a 100644 --- a/pkg/storage/unified/sql/backend.go +++ b/pkg/storage/unified/sql/backend.go @@ -11,7 +11,6 @@ import ( "github.com/go-sql-driver/mysql" "github.com/google/uuid" - unifiedbackend "github.com/grafana/grafana/pkg/storage/unified/backend" "github.com/jackc/pgx/v5/pgconn" "github.com/lib/pq" "github.com/mattn/go-sqlite3" @@ -22,6 +21,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "github.com/grafana/grafana/pkg/infra/log" + unifiedbackend "github.com/grafana/grafana/pkg/storage/unified/backend" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/sql/db" "github.com/grafana/grafana/pkg/storage/unified/sql/dbutil" @@ -353,6 +353,7 @@ func (b *backend) create(ctx context.Context, event resource.WriteEvent) (int64, SQLTemplate: sqltemplate.New(b.dialect), WriteEvent: event, Folder: folder, + Generation: event.Object.GetGeneration(), GUID: guid, }); err != nil { return guid, fmt.Errorf("insert into resource history: %w", err) @@ -441,6 +442,7 @@ func (b *backend) update(ctx context.Context, event resource.WriteEvent) (int64, WriteEvent: event, Folder: folder, GUID: guid, + Generation: event.Object.GetGeneration(), }); err != nil { return guid, fmt.Errorf("insert into resource history: %w", err) } @@ -494,6 +496,7 @@ func (b *backend) delete(ctx context.Context, event resource.WriteEvent) (int64, WriteEvent: event, Folder: folder, GUID: guid, + Generation: 0, // object does not exist }); err != nil { return guid, fmt.Errorf("insert into resource history: %w", err) } diff --git a/pkg/storage/unified/sql/data/resource_history_insert.sql b/pkg/storage/unified/sql/data/resource_history_insert.sql index b6dca731270..4ac3cc03547 100644 --- a/pkg/storage/unified/sql/data/resource_history_insert.sql +++ b/pkg/storage/unified/sql/data/resource_history_insert.sql @@ -10,6 +10,7 @@ INSERT INTO {{ .Ident "resource_history" }} {{ .Ident "resource_version" }}, {{ end }} {{ .Ident "previous_resource_version"}}, + {{ .Ident "generation"}}, {{ .Ident "value" }}, {{ .Ident "action" }} ) @@ -25,6 +26,7 @@ INSERT INTO {{ .Ident "resource_history" }} {{ .Arg .ResourceVersion }}, {{ end }} {{ .Arg .WriteEvent.PreviousRV }}, + {{ .Arg .Generation }}, {{ .Arg .WriteEvent.Value }}, {{ .Arg .WriteEvent.Type }} ) diff --git a/pkg/storage/unified/sql/data/resource_history_prune.sql b/pkg/storage/unified/sql/data/resource_history_prune.sql index 4bff037876a..c2dca011998 100644 --- a/pkg/storage/unified/sql/data/resource_history_prune.sql +++ b/pkg/storage/unified/sql/data/resource_history_prune.sql @@ -5,19 +5,23 @@ WHERE {{ .Ident "guid" }} IN ( SELECT {{ .Ident "guid" }}, ROW_NUMBER() OVER ( - PARTITION BY - {{ .Ident "namespace" }}, - {{ .Ident "group" }}, - {{ .Ident "resource" }}, - {{ .Ident "name" }} + PARTITION BY {{ .Ident "namespace" }} + , {{ .Ident "group" }} + , {{ .Ident "resource" }} + , {{ .Ident "name" }} + {{ if .PartitionByGeneration }} + , {{ .Ident "generation" }} + {{ end }} ORDER BY {{ .Ident "resource_version" }} DESC ) AS {{ .Ident "rn" }} FROM {{ .Ident "resource_history" }} - WHERE - {{ .Ident "namespace" }} = {{ .Arg .Key.Namespace }} + WHERE {{ .Ident "namespace" }} = {{ .Arg .Key.Namespace }} AND {{ .Ident "group" }} = {{ .Arg .Key.Group }} AND {{ .Ident "resource" }} = {{ .Arg .Key.Resource }} AND {{ .Ident "name" }} = {{ .Arg .Key.Name }} + {{ if .PartitionByGeneration }} + AND {{ .Ident "generation" }} > 0 + {{ end }} ) AS {{ .Ident "ranked" }} WHERE {{ .Ident "rn" }} > {{ .Arg .HistoryLimit }} ); diff --git a/pkg/storage/unified/sql/db/migrations/resource_mig.go b/pkg/storage/unified/sql/db/migrations/resource_mig.go index 90da4763e68..6bd32b5a0d8 100644 --- a/pkg/storage/unified/sql/db/migrations/resource_mig.go +++ b/pkg/storage/unified/sql/db/migrations/resource_mig.go @@ -152,7 +152,19 @@ func initResourceTables(mg *migrator.Migrator) string { mg.AddMigration("Migrate DeletionMarkers to real Resource objects", &deletionMarkerMigrator{}) mg.AddMigration("Add index to resource_history for get trash", migrator.NewAddIndexMigration(resource_history_table, &migrator.Index{ - Name: "IDX_resource_history_namespace_group_resource_action_version", Cols: []string{"namespace", "group", "resource", "action", "resource_version"}, Type: migrator.IndexType, + Name: "IDX_resource_history_namespace_group_resource_action_version", + Cols: []string{"namespace", "group", "resource", "action", "resource_version"}, + Type: migrator.IndexType, + })) + + // Add generation column so we can use it for more aggressive pruning + mg.AddMigration("Add generation to resource history", migrator.NewAddColumnMigration(resource_history_table, &migrator.Column{ + Name: "generation", Type: migrator.DB_BigInt, Nullable: false, Default: "0", + })) + mg.AddMigration("Add generation index to resource history", migrator.NewAddIndexMigration(resource_history_table, &migrator.Index{ + Cols: []string{"namespace", "group", "resource", "name", "generation"}, + Type: migrator.IndexType, + Name: "IDX_resource_history_namespace_group_resource_name_generation", })) return marker diff --git a/pkg/storage/unified/sql/queries.go b/pkg/storage/unified/sql/queries.go index 758cff6fcec..d3592225199 100644 --- a/pkg/storage/unified/sql/queries.go +++ b/pkg/storage/unified/sql/queries.go @@ -75,6 +75,7 @@ type sqlResourceRequest struct { sqltemplate.SQLTemplate GUID string WriteEvent resource.WriteEvent + Generation int64 Folder string // Useful when batch writing @@ -305,8 +306,9 @@ func (r sqlGetHistoryRequest) Validate() error { // prune resource history type sqlPruneHistoryRequest struct { sqltemplate.SQLTemplate - Key *resource.ResourceKey - HistoryLimit int64 + Key *resource.ResourceKey + PartitionByGeneration bool // include generation in the partition + HistoryLimit int64 } func (r *sqlPruneHistoryRequest) Validate() error { diff --git a/pkg/storage/unified/sql/queries_test.go b/pkg/storage/unified/sql/queries_test.go index 69697e8fcfd..a3f82de080e 100644 --- a/pkg/storage/unified/sql/queries_test.go +++ b/pkg/storage/unified/sql/queries_test.go @@ -215,6 +215,7 @@ func TestUnifiedStorageQueries(t *testing.T) { Name: "insert into resource_history", Data: &sqlResourceRequest{ SQLTemplate: mocks.NewTestingSQLTemplate(), + Generation: 789, WriteEvent: resource.WriteEvent{ Key: &resource.ResourceKey{ Namespace: "nn", @@ -271,16 +272,30 @@ func TestUnifiedStorageQueries(t *testing.T) { sqlResourceHistoryPrune: { { - Name: "simple", + Name: "max-versions", Data: &sqlPruneHistoryRequest{ SQLTemplate: mocks.NewTestingSQLTemplate(), Key: &resource.ResourceKey{ - Namespace: "nn", - Group: "gg", - Resource: "rr", - Name: "na", + Namespace: "default", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "repo-xyz", }, - HistoryLimit: 100, + HistoryLimit: 10, + }, + }, + { + Name: "collapse-generations", + Data: &sqlPruneHistoryRequest{ + SQLTemplate: mocks.NewTestingSQLTemplate(), + Key: &resource.ResourceKey{ + Namespace: "default", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "repo-xyz", + }, + PartitionByGeneration: true, + HistoryLimit: 1, }, }, }, diff --git a/pkg/storage/unified/sql/testdata/mysql--resource_history_insert-insert into resource_history.sql b/pkg/storage/unified/sql/testdata/mysql--resource_history_insert-insert into resource_history.sql index 4f3d859d886..6eae2b07b7a 100755 --- a/pkg/storage/unified/sql/testdata/mysql--resource_history_insert-insert into resource_history.sql +++ b/pkg/storage/unified/sql/testdata/mysql--resource_history_insert-insert into resource_history.sql @@ -7,6 +7,7 @@ INSERT INTO `resource_history` `name`, `folder`, `previous_resource_version`, + `generation`, `value`, `action` ) @@ -18,6 +19,7 @@ INSERT INTO `resource_history` 'name', 'fldr', 1234, + 789, '[]', 'UNKNOWN' ) diff --git a/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-collapse-generations.sql b/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-collapse-generations.sql new file mode 100755 index 00000000000..7992ffe9396 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-collapse-generations.sql @@ -0,0 +1,23 @@ +DELETE FROM `resource_history` +WHERE `guid` IN ( + SELECT `guid` + FROM ( + SELECT + `guid`, + ROW_NUMBER() OVER ( + PARTITION BY `namespace` + , `group` + , `resource` + , `name` + , `generation` + ORDER BY `resource_version` DESC + ) AS `rn` + FROM `resource_history` + WHERE `namespace` = 'default' + AND `group` = 'provisioning.grafana.app' + AND `resource` = 'repositories' + AND `name` = 'repo-xyz' + AND `generation` > 0 + ) AS `ranked` + WHERE `rn` > 1 +); diff --git a/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-max-versions.sql b/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-max-versions.sql new file mode 100755 index 00000000000..928680ebb95 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-max-versions.sql @@ -0,0 +1,21 @@ +DELETE FROM `resource_history` +WHERE `guid` IN ( + SELECT `guid` + FROM ( + SELECT + `guid`, + ROW_NUMBER() OVER ( + PARTITION BY `namespace` + , `group` + , `resource` + , `name` + ORDER BY `resource_version` DESC + ) AS `rn` + FROM `resource_history` + WHERE `namespace` = 'default' + AND `group` = 'provisioning.grafana.app' + AND `resource` = 'repositories' + AND `name` = 'repo-xyz' + ) AS `ranked` + WHERE `rn` > 10 +); diff --git a/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-simple.sql b/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-simple.sql deleted file mode 100755 index 2c54050b2d2..00000000000 --- a/pkg/storage/unified/sql/testdata/mysql--resource_history_prune-simple.sql +++ /dev/null @@ -1,23 +0,0 @@ -DELETE FROM `resource_history` -WHERE `guid` IN ( - SELECT `guid` - FROM ( - SELECT - `guid`, - ROW_NUMBER() OVER ( - PARTITION BY - `namespace`, - `group`, - `resource`, - `name` - ORDER BY `resource_version` DESC - ) AS `rn` - FROM `resource_history` - WHERE - `namespace` = 'nn' - AND `group` = 'gg' - AND `resource` = 'rr' - AND `name` = 'na' - ) AS `ranked` - WHERE `rn` > 100 -); diff --git a/pkg/storage/unified/sql/testdata/postgres--resource_history_insert-insert into resource_history.sql b/pkg/storage/unified/sql/testdata/postgres--resource_history_insert-insert into resource_history.sql index 6014421cd7f..11f6b18c01b 100755 --- a/pkg/storage/unified/sql/testdata/postgres--resource_history_insert-insert into resource_history.sql +++ b/pkg/storage/unified/sql/testdata/postgres--resource_history_insert-insert into resource_history.sql @@ -7,6 +7,7 @@ INSERT INTO "resource_history" "name", "folder", "previous_resource_version", + "generation", "value", "action" ) @@ -18,6 +19,7 @@ INSERT INTO "resource_history" 'name', 'fldr', 1234, + 789, '[]', 'UNKNOWN' ) diff --git a/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-collapse-generations.sql b/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-collapse-generations.sql new file mode 100755 index 00000000000..1421961b5e7 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-collapse-generations.sql @@ -0,0 +1,23 @@ +DELETE FROM "resource_history" +WHERE "guid" IN ( + SELECT "guid" + FROM ( + SELECT + "guid", + ROW_NUMBER() OVER ( + PARTITION BY "namespace" + , "group" + , "resource" + , "name" + , "generation" + ORDER BY "resource_version" DESC + ) AS "rn" + FROM "resource_history" + WHERE "namespace" = 'default' + AND "group" = 'provisioning.grafana.app' + AND "resource" = 'repositories' + AND "name" = 'repo-xyz' + AND "generation" > 0 + ) AS "ranked" + WHERE "rn" > 1 +); diff --git a/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-max-versions.sql b/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-max-versions.sql new file mode 100755 index 00000000000..68398964b83 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-max-versions.sql @@ -0,0 +1,21 @@ +DELETE FROM "resource_history" +WHERE "guid" IN ( + SELECT "guid" + FROM ( + SELECT + "guid", + ROW_NUMBER() OVER ( + PARTITION BY "namespace" + , "group" + , "resource" + , "name" + ORDER BY "resource_version" DESC + ) AS "rn" + FROM "resource_history" + WHERE "namespace" = 'default' + AND "group" = 'provisioning.grafana.app' + AND "resource" = 'repositories' + AND "name" = 'repo-xyz' + ) AS "ranked" + WHERE "rn" > 10 +); diff --git a/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-simple.sql b/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-simple.sql deleted file mode 100755 index 08a36ac095a..00000000000 --- a/pkg/storage/unified/sql/testdata/postgres--resource_history_prune-simple.sql +++ /dev/null @@ -1,23 +0,0 @@ -DELETE FROM "resource_history" -WHERE "guid" IN ( - SELECT "guid" - FROM ( - SELECT - "guid", - ROW_NUMBER() OVER ( - PARTITION BY - "namespace", - "group", - "resource", - "name" - ORDER BY "resource_version" DESC - ) AS "rn" - FROM "resource_history" - WHERE - "namespace" = 'nn' - AND "group" = 'gg' - AND "resource" = 'rr' - AND "name" = 'na' - ) AS "ranked" - WHERE "rn" > 100 -); diff --git a/pkg/storage/unified/sql/testdata/sqlite--resource_history_insert-insert into resource_history.sql b/pkg/storage/unified/sql/testdata/sqlite--resource_history_insert-insert into resource_history.sql index 6014421cd7f..11f6b18c01b 100755 --- a/pkg/storage/unified/sql/testdata/sqlite--resource_history_insert-insert into resource_history.sql +++ b/pkg/storage/unified/sql/testdata/sqlite--resource_history_insert-insert into resource_history.sql @@ -7,6 +7,7 @@ INSERT INTO "resource_history" "name", "folder", "previous_resource_version", + "generation", "value", "action" ) @@ -18,6 +19,7 @@ INSERT INTO "resource_history" 'name', 'fldr', 1234, + 789, '[]', 'UNKNOWN' ) diff --git a/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-collapse-generations.sql b/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-collapse-generations.sql new file mode 100755 index 00000000000..1421961b5e7 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-collapse-generations.sql @@ -0,0 +1,23 @@ +DELETE FROM "resource_history" +WHERE "guid" IN ( + SELECT "guid" + FROM ( + SELECT + "guid", + ROW_NUMBER() OVER ( + PARTITION BY "namespace" + , "group" + , "resource" + , "name" + , "generation" + ORDER BY "resource_version" DESC + ) AS "rn" + FROM "resource_history" + WHERE "namespace" = 'default' + AND "group" = 'provisioning.grafana.app' + AND "resource" = 'repositories' + AND "name" = 'repo-xyz' + AND "generation" > 0 + ) AS "ranked" + WHERE "rn" > 1 +); diff --git a/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-max-versions.sql b/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-max-versions.sql new file mode 100755 index 00000000000..68398964b83 --- /dev/null +++ b/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-max-versions.sql @@ -0,0 +1,21 @@ +DELETE FROM "resource_history" +WHERE "guid" IN ( + SELECT "guid" + FROM ( + SELECT + "guid", + ROW_NUMBER() OVER ( + PARTITION BY "namespace" + , "group" + , "resource" + , "name" + ORDER BY "resource_version" DESC + ) AS "rn" + FROM "resource_history" + WHERE "namespace" = 'default' + AND "group" = 'provisioning.grafana.app' + AND "resource" = 'repositories' + AND "name" = 'repo-xyz' + ) AS "ranked" + WHERE "rn" > 10 +); diff --git a/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-simple.sql b/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-simple.sql deleted file mode 100755 index 08a36ac095a..00000000000 --- a/pkg/storage/unified/sql/testdata/sqlite--resource_history_prune-simple.sql +++ /dev/null @@ -1,23 +0,0 @@ -DELETE FROM "resource_history" -WHERE "guid" IN ( - SELECT "guid" - FROM ( - SELECT - "guid", - ROW_NUMBER() OVER ( - PARTITION BY - "namespace", - "group", - "resource", - "name" - ORDER BY "resource_version" DESC - ) AS "rn" - FROM "resource_history" - WHERE - "namespace" = 'nn' - AND "group" = 'gg' - AND "resource" = 'rr' - AND "name" = 'na' - ) AS "ranked" - WHERE "rn" > 100 -); diff --git a/pkg/tests/api/dashboards/api_dashboards_test.go b/pkg/tests/api/dashboards/api_dashboards_test.go index f188bed455a..ee84059a4e6 100644 --- a/pkg/tests/api/dashboards/api_dashboards_test.go +++ b/pkg/tests/api/dashboards/api_dashboards_test.go @@ -236,7 +236,7 @@ providers: return retryer.FuncFailure, nil } return retryer.FuncComplete, nil - }, retries, time.Millisecond*time.Duration(10), time.Second) + }, retries, time.Millisecond*time.Duration(25), time.Second) require.NoError(t, err) var dashboardUID string