Skip to content

Commit

Permalink
Storage: Customise setting dual writing modes (#87668)
Browse files Browse the repository at this point in the history
* Add feature toggles for mode 2 and 3 playlist dual writing
* Make current mode customised based on kind
* Check feature flags when initialising dual writer
* Fix linting
* Refactor NewDualWriter
  • Loading branch information
suntala committed May 14, 2024
1 parent d8904f3 commit 6836bfe
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 41 deletions.
1 change: 1 addition & 0 deletions .golangci.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ allow = [
"github.com/grafana/grafana/pkg/apimachinery",
"github.com/grafana/grafana/pkg/apiserver",
"github.com/grafana/grafana/pkg/services/apiserver/utils",
"github.com/grafana/grafana/pkg/services/featuremgmt",
]
deny = [
{ pkg = "github.com/grafana/grafana/pkg", desc = "apiserver is not allowed to import grafana core" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ Experimental features might be changed or removed without prior notice.
| `disableSecretsCompatibility` | Disable duplicated secret storage in legacy tables |
| `logRequestsInstrumentedAsUnknown` | Logs the path for requests that are instrumented as unknown |
| `unifiedStorage` | SQL-based k8s storage |
| `dualWritePlaylistsMode2` | Enables dual writing of playlists to both legacy and k8s storage in mode 2 |
| `dualWritePlaylistsMode3` | Enables dual writing of playlists to both legacy and k8s storage in mode 3 |
| `showDashboardValidationWarnings` | Show warnings when dashboards do not validate against the schema |
| `mysqlAnsiQuotes` | Use double quotes to escape keyword in a MySQL query |
| `alertingBacktesting` | Rule backtesting API for alerting |
Expand Down
2 changes: 2 additions & 0 deletions packages/grafana-data/src/types/featureToggles.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export interface FeatureToggles {
returnToPrevious?: boolean;
grpcServer?: boolean;
unifiedStorage?: boolean;
dualWritePlaylistsMode2?: boolean;
dualWritePlaylistsMode3?: boolean;
cloudWatchCrossAccountQuerying?: boolean;
showDashboardValidationWarnings?: boolean;
mysqlAnsiQuotes?: boolean;
Expand Down
43 changes: 17 additions & 26 deletions pkg/apiserver/rest/dualwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,24 @@ const (
Mode4
)

var CurrentMode = Mode2

//TODO: make CurrentMode customisable and specific to each entity
// change DualWriter signature to get the current mode as an argument

// NewDualWriter returns a new DualWriter.
func NewDualWriter(legacy LegacyStorage, storage Storage) DualWriter {
return selectDualWriter(CurrentMode, legacy, storage)
func NewDualWriter(mode DualWriterMode, legacy LegacyStorage, storage Storage) DualWriter {
switch mode {
case Mode1:
// read and write only from legacy storage
return NewDualWriterMode1(legacy, storage)
case Mode2:
// write to both, read from storage but use legacy as backup
return NewDualWriterMode2(legacy, storage)
case Mode3:
// write to both, read from storage only
return NewDualWriterMode3(legacy, storage)
case Mode4:
// read and write only from storage
return NewDualWriterMode4(legacy, storage)
default:
return NewDualWriterMode1(legacy, storage)
}
}

type updateWrapper struct {
Expand All @@ -107,22 +117,3 @@ func (u *updateWrapper) Preconditions() *metav1.Preconditions {
func (u *updateWrapper) UpdatedObject(ctx context.Context, oldObj runtime.Object) (newObj runtime.Object, err error) {
return u.updated, nil
}

func selectDualWriter(mode DualWriterMode, legacy LegacyStorage, storage Storage) DualWriter {
switch mode {
case Mode1:
// read and write only from legacy storage
return NewDualWriterMode1(legacy, storage)
case Mode2:
// write to both, read from storage but use legacy as backup
return NewDualWriterMode2(legacy, storage)
case Mode3:
// write to both, read from storage only
return NewDualWriterMode3(legacy, storage)
case Mode4:
// read and write only from storage
return NewDualWriterMode4(legacy, storage)
default:
return NewDualWriterMode1(legacy, storage)
}
}
12 changes: 6 additions & 6 deletions pkg/apiserver/rest/dualwriter_mode1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestMode1_Create(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

obj, err := dw.Create(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{})

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestMode1_Get(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{})

Expand Down Expand Up @@ -175,7 +175,7 @@ func TestMode1_List(t *testing.T) {
tt.setupStorageFn(m)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

_, err := dw.List(context.Background(), &metainternalversion.ListOptions{})

Expand Down Expand Up @@ -228,7 +228,7 @@ func TestMode1_Delete(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

obj, _, err := dw.Delete(context.Background(), tt.input, func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{})

Expand Down Expand Up @@ -285,7 +285,7 @@ func TestMode1_DeleteCollection(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

obj, err := dw.DeleteCollection(context.Background(), func(ctx context.Context, obj runtime.Object) error { return nil }, tt.input, &metainternalversion.ListOptions{})

Expand Down Expand Up @@ -359,7 +359,7 @@ func TestMode1_Update(t *testing.T) {
tt.setupGetFn(m, tt.input)
}

dw := selectDualWriter(Mode1, ls, us)
dw := NewDualWriter(Mode1, ls, us)

obj, _, err := dw.Update(context.Background(), tt.input, UpdatedObjInfoObj{}, func(ctx context.Context, obj runtime.Object) error { return nil }, func(ctx context.Context, obj, old runtime.Object) error { return nil }, false, &metav1.UpdateOptions{})

Expand Down
12 changes: 6 additions & 6 deletions pkg/apiserver/rest/dualwriter_mode2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestMode2_Create(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, err := dw.Create(context.Background(), tt.input, createFn, &metav1.CreateOptions{})

Expand Down Expand Up @@ -138,7 +138,7 @@ func TestMode2_Get(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{})

Expand Down Expand Up @@ -189,7 +189,7 @@ func TestMode2_List(t *testing.T) {
tt.setupStorageFn(m)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, err := dw.List(context.Background(), &metainternalversion.ListOptions{})

Expand Down Expand Up @@ -281,7 +281,7 @@ func TestMode2_Delete(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, _, err := dw.Delete(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.DeleteOptions{})

Expand Down Expand Up @@ -351,7 +351,7 @@ func TestMode2_DeleteCollection(t *testing.T) {
tt.setupStorageFn(m)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, err := dw.DeleteCollection(context.Background(), func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: tt.input}}, &metainternalversion.ListOptions{})

Expand Down Expand Up @@ -458,7 +458,7 @@ func TestMode2_Update(t *testing.T) {
tt.setupStorageFn(m, tt.input)
}

dw := selectDualWriter(Mode2, ls, us)
dw := NewDualWriter(Mode2, ls, us)

obj, _, err := dw.Update(context.Background(), tt.input, UpdatedObjInfoObj{}, func(ctx context.Context, obj runtime.Object) error { return nil }, func(ctx context.Context, obj, old runtime.Object) error { return nil }, false, &metav1.UpdateOptions{})

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/apis/dashboard/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (b *DashboardsAPIBuilder) GetAPIGroupInfo(
if err := store.CompleteWithOptions(options); err != nil {
return nil, err
}
storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(legacyStore, store)
storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(grafanarest.Mode1, legacyStore, store)
}

// Summary
Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/apis/folders/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (b *FolderAPIBuilder) GetAPIGroupInfo(
if err != nil {
return nil, err
}
storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(legacyStore, store)
storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(grafanarest.Mode1, legacyStore, store)
}

apiGroupInfo.VersionedResourcesStorageMap[v0alpha1.VERSION] = storage
Expand Down
11 changes: 10 additions & 1 deletion pkg/registry/apis/playlist/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
"github.com/grafana/grafana/pkg/services/apiserver/utils"
"github.com/grafana/grafana/pkg/services/featuremgmt"
playlistsvc "github.com/grafana/grafana/pkg/services/playlist"
"github.com/grafana/grafana/pkg/setting"
)
Expand All @@ -30,16 +31,19 @@ type PlaylistAPIBuilder struct {
service playlistsvc.Service
namespacer request.NamespaceMapper
gv schema.GroupVersion
features featuremgmt.FeatureToggles
}

func RegisterAPIService(p playlistsvc.Service,
apiregistration builder.APIRegistrar,
cfg *setting.Cfg,
features featuremgmt.FeatureToggles,
) *PlaylistAPIBuilder {
builder := &PlaylistAPIBuilder{
service: p,
namespacer: request.GetNamespaceMapper(cfg),
gv: playlist.PlaylistResourceInfo.GroupVersion(),
features: features,
}
apiregistration.RegisterAPI(builder)
return builder
Expand Down Expand Up @@ -118,7 +122,12 @@ func (b *PlaylistAPIBuilder) GetAPIGroupInfo(
if err != nil {
return nil, err
}
storage[resource.StoragePath()] = grafanarest.NewDualWriterMode2(legacyStore, store)

mode := grafanarest.Mode1
if b.features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode2) {
mode = grafanarest.Mode2
}
storage[resource.StoragePath()] = grafanarest.NewDualWriter(mode, legacyStore, store)
}

apiGroupInfo.VersionedResourcesStorageMap[playlist.VERSION] = storage
Expand Down
12 changes: 12 additions & 0 deletions pkg/services/featuremgmt/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ var (
RequiresRestart: true, // new SQL tables created
Owner: grafanaAppPlatformSquad,
},
{
Name: "dualWritePlaylistsMode2",
Description: "Enables dual writing of playlists to both legacy and k8s storage in mode 2",
Stage: FeatureStageExperimental,
Owner: grafanaSearchAndStorageSquad,
},
{
Name: "dualWritePlaylistsMode3",
Description: "Enables dual writing of playlists to both legacy and k8s storage in mode 3",
Stage: FeatureStageExperimental,
Owner: grafanaSearchAndStorageSquad,
},
{
Name: "cloudWatchCrossAccountQuerying",
Description: "Enables cross-account querying in CloudWatch datasources",
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/featuremgmt/toggles_gen.csv
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ topnav,deprecated,@grafana/grafana-frontend-platform,false,false,false
returnToPrevious,GA,@grafana/grafana-frontend-platform,false,false,true
grpcServer,preview,@grafana/grafana-app-platform-squad,false,false,false
unifiedStorage,experimental,@grafana/grafana-app-platform-squad,false,true,false
dualWritePlaylistsMode2,experimental,@grafana/search-and-storage,false,false,false
dualWritePlaylistsMode3,experimental,@grafana/search-and-storage,false,false,false
cloudWatchCrossAccountQuerying,GA,@grafana/aws-datasources,false,false,false
showDashboardValidationWarnings,experimental,@grafana/dashboards-squad,false,false,false
mysqlAnsiQuotes,experimental,@grafana/search-and-storage,false,false,false
Expand Down
8 changes: 8 additions & 0 deletions pkg/services/featuremgmt/toggles_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ const (
// SQL-based k8s storage
FlagUnifiedStorage = "unifiedStorage"

// FlagDualWritePlaylistsMode2
// Enables dual writing of playlists to both legacy and k8s storage in mode 2
FlagDualWritePlaylistsMode2 = "dualWritePlaylistsMode2"

// FlagDualWritePlaylistsMode3
// Enables dual writing of playlists to both legacy and k8s storage in mode 3
FlagDualWritePlaylistsMode3 = "dualWritePlaylistsMode3"

// FlagCloudWatchCrossAccountQuerying
// Enables cross-account querying in CloudWatch datasources
FlagCloudWatchCrossAccountQuerying = "cloudWatchCrossAccountQuerying"
Expand Down
24 changes: 24 additions & 0 deletions pkg/services/featuremgmt/toggles_gen.json
Original file line number Diff line number Diff line change
Expand Up @@ -2179,6 +2179,30 @@
"stage": "experimental",
"codeowner": "@grafana/grafana-frontend-platform"
}
},
{
"metadata": {
"name": "dualWritePlaylistsMode2",
"resourceVersion": "1715369873132",
"creationTimestamp": "2024-05-10T19:37:53Z"
},
"spec": {
"description": "Enables dual writing of playlists to both legacy and k8s storage in mode 2",
"stage": "experimental",
"codeowner": "@grafana/search-and-storage"
}
},
{
"metadata": {
"name": "dualWritePlaylistsMode3",
"resourceVersion": "1715369873132",
"creationTimestamp": "2024-05-10T19:37:53Z"
},
"spec": {
"description": "Enables dual writing of playlists to both legacy and k8s storage in mode 3",
"stage": "experimental",
"codeowner": "@grafana/search-and-storage"
}
}
]
}

0 comments on commit 6836bfe

Please sign in to comment.