From 359fb12a1335b41f940d43b5b304b2c919d05128 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Tue, 5 Aug 2025 16:08:40 +0100 Subject: [PATCH] implement context aware instance manager selection with new TTL manager --- backend/app/instance_provider.go | 6 +- backend/datasource/instance_provider.go | 2 +- backend/instancemgmt/context_aware_manager.go | 53 +++ .../context_aware_manager_test.go | 115 ++++++ backend/instancemgmt/doc.go | 17 + backend/instancemgmt/instance_manager.go | 37 +- backend/instancemgmt/instance_manager_test.go | 19 +- backend/instancemgmt/intance_manager_ttl.go | 122 +++++++ .../instancemgmt/intance_manager_ttl_test.go | 332 ++++++++++++++++++ experimental/featuretoggles/featuretoggles.go | 4 + go.mod | 1 + go.sum | 2 + 12 files changed, 684 insertions(+), 26 deletions(-) create mode 100644 backend/instancemgmt/context_aware_manager.go create mode 100644 backend/instancemgmt/context_aware_manager_test.go create mode 100644 backend/instancemgmt/intance_manager_ttl.go create mode 100644 backend/instancemgmt/intance_manager_ttl_test.go diff --git a/backend/app/instance_provider.go b/backend/app/instance_provider.go index a4dfdaa05..769d23c3c 100644 --- a/backend/app/instance_provider.go +++ b/backend/app/instance_provider.go @@ -13,13 +13,13 @@ import ( // InstanceFactoryFunc factory method for creating app instances. type InstanceFactoryFunc func(ctx context.Context, settings backend.AppInstanceSettings) (instancemgmt.Instance, error) -// NewInstanceManager creates a new app instance manager, +// NewInstanceManager creates a new app instance manager. // // This is a helper method for calling NewInstanceProvider and creating a new instancemgmt.InstanceProvider, -// and providing that to instancemgmt.New. +// and providing that to instancemgmt.NewInstanceManagerWrapper. func NewInstanceManager(fn InstanceFactoryFunc) instancemgmt.InstanceManager { ip := NewInstanceProvider(fn) - return instancemgmt.New(ip) + return instancemgmt.NewInstanceManagerWrapper(ip) } // NewInstanceProvider create a new app instance provider, diff --git a/backend/datasource/instance_provider.go b/backend/datasource/instance_provider.go index 1c34bc4d5..f8f26a64f 100644 --- a/backend/datasource/instance_provider.go +++ b/backend/datasource/instance_provider.go @@ -30,7 +30,7 @@ type InstanceFactoryFunc func(ctx context.Context, settings backend.DataSourceIn // and providing that to instancemgmt.New. func NewInstanceManager(fn InstanceFactoryFunc) instancemgmt.InstanceManager { ip := NewInstanceProvider(fn) - return instancemgmt.New(ip) + return instancemgmt.NewInstanceManagerWrapper(ip) } // NewInstanceProvider create a new data source instance provuder, diff --git a/backend/instancemgmt/context_aware_manager.go b/backend/instancemgmt/context_aware_manager.go new file mode 100644 index 000000000..7d2f195da --- /dev/null +++ b/backend/instancemgmt/context_aware_manager.go @@ -0,0 +1,53 @@ +package instancemgmt + +import ( + "context" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/experimental/featuretoggles" +) + +// NewInstanceManagerWrapper creates a new instance manager that dynamically selects +// between standard and TTL instance managers based on feature toggles from the Grafana config. +func NewInstanceManagerWrapper(provider InstanceProvider) InstanceManager { + return &instanceManagerWrapper{ + provider: provider, + standardManager: New(provider), + ttlManager: NewTTLInstanceManager(provider), + } +} + +// instanceManagerWrapper is a wrapper that dynamically selects the appropriate +// instance manager implementation based on feature toggles in the context. +type instanceManagerWrapper struct { + provider InstanceProvider + standardManager InstanceManager + ttlManager InstanceManager +} + +// selectManager returns the appropriate instance manager based on the feature toggle +// from the Grafana config in the plugin context. +func (c *instanceManagerWrapper) selectManager(_ context.Context, pluginContext backend.PluginContext) InstanceManager { + // Check if TTL instance manager feature toggle is enabled + if pluginContext.GrafanaConfig != nil { + featureToggles := pluginContext.GrafanaConfig.FeatureToggles() + if featureToggles.IsEnabled(featuretoggles.TTLInstanceManager) { + return c.ttlManager + } + } + + // Default to standard instance manager + return c.standardManager +} + +// Get returns an Instance using the appropriate manager based on feature toggles. +func (c *instanceManagerWrapper) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) { + manager := c.selectManager(ctx, pluginContext) + return manager.Get(ctx, pluginContext) +} + +// Do provides an Instance as argument to fn using the appropriate manager based on feature toggles. +func (c *instanceManagerWrapper) Do(ctx context.Context, pluginContext backend.PluginContext, fn InstanceCallbackFunc) error { + manager := c.selectManager(ctx, pluginContext) + return manager.Do(ctx, pluginContext, fn) +} diff --git a/backend/instancemgmt/context_aware_manager_test.go b/backend/instancemgmt/context_aware_manager_test.go new file mode 100644 index 000000000..56e947e6d --- /dev/null +++ b/backend/instancemgmt/context_aware_manager_test.go @@ -0,0 +1,115 @@ +package instancemgmt + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/experimental/featuretoggles" +) + +func TestInstanceManagerWrapper(t *testing.T) { + ctx := context.Background() + tip := &testInstanceProvider{} + im := NewInstanceManagerWrapper(tip) + + t.Run("Should use standard manager when feature toggle is disabled", func(t *testing.T) { + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: backend.NewGrafanaCfg(map[string]string{ + featuretoggles.EnabledFeatures: "", + }), + } + + manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx) + require.IsType(t, &instanceManager{}, manager) + }) + + t.Run("Should use TTL manager when feature toggle is enabled", func(t *testing.T) { + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: backend.NewGrafanaCfg(map[string]string{ + featuretoggles.EnabledFeatures: featuretoggles.TTLInstanceManager, + }), + } + + manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx) + require.IsType(t, &instanceManagerWithTTL{}, manager) + }) + + t.Run("Should use standard manager when GrafanaConfig is nil", func(t *testing.T) { + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: nil, + } + + manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx) + require.IsType(t, &instanceManager{}, manager) + }) + + t.Run("Should use TTL manager when feature toggle is enabled with other flags", func(t *testing.T) { + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: backend.NewGrafanaCfg(map[string]string{ + featuretoggles.EnabledFeatures: "someOtherFlag," + featuretoggles.TTLInstanceManager + ",anotherFlag", + }), + } + + manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx) + require.IsType(t, &instanceManagerWithTTL{}, manager) + }) + + t.Run("Should delegate Get calls correctly", func(t *testing.T) { + // Test with TTL manager enabled + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: backend.NewGrafanaCfg(map[string]string{ + featuretoggles.EnabledFeatures: featuretoggles.TTLInstanceManager, + }), + } + + instance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, instance) + require.Equal(t, pCtx.OrgID, instance.(*testInstance).orgID) + }) + + t.Run("Should delegate Do calls correctly", func(t *testing.T) { + // Test with standard manager (no feature toggle) + pCtx := backend.PluginContext{ + OrgID: 2, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + GrafanaConfig: backend.NewGrafanaCfg(map[string]string{ + featuretoggles.EnabledFeatures: "", + }), + } + + var receivedInstance *testInstance + err := im.Do(ctx, pCtx, func(instance *testInstance) { + receivedInstance = instance + }) + require.NoError(t, err) + require.NotNil(t, receivedInstance) + require.Equal(t, pCtx.OrgID, receivedInstance.orgID) + }) +} diff --git a/backend/instancemgmt/doc.go b/backend/instancemgmt/doc.go index 23cde79fb..b4f448994 100644 --- a/backend/instancemgmt/doc.go +++ b/backend/instancemgmt/doc.go @@ -1,2 +1,19 @@ // Package instancemgmt provides utilities for managing plugin instances. +// +// This package offers several instance manager implementations: +// +// 1. Standard Instance Manager (New): Uses sync.Map for caching instances +// and disposes them when they need updates. +// +// 2. TTL Instance Manager (NewTTLInstanceManager): Uses TTL-based caching +// that automatically evicts instances after a configurable time period. +// +// 3. Instance Manager Wrapper (NewInstanceManagerWrapper): +// Dynamically selects between standard and TTL managers based on +// feature toggles from the Grafana config in the context. +// +// The context-aware manager checks the "ttlInstanceManager" feature toggle +// from the Grafana configuration and automatically uses the appropriate +// underlying implementation. This allows runtime switching without requiring +// plugin restarts or static configuration. package instancemgmt diff --git a/backend/instancemgmt/instance_manager.go b/backend/instancemgmt/instance_manager.go index 4ca761d81..9697a812a 100644 --- a/backend/instancemgmt/instance_manager.go +++ b/backend/instancemgmt/instance_manager.go @@ -18,9 +18,10 @@ var ( Name: "active_instances", Help: "The number of active plugin instances", }) - disposeTTL = 5 * time.Second ) +const defaultDisposeTTL = 5 * time.Second // Time to wait before disposing an instance + // Instance is a marker interface for an instance. type Instance interface{} @@ -73,21 +74,33 @@ type InstanceProvider interface { // New create a new instance manager. func New(provider InstanceProvider) InstanceManager { + return newInstanceManager(provider, defaultDisposeTTL) +} + +func newInstanceManager(provider InstanceProvider, disposeTTL time.Duration) *instanceManager { if provider == nil { panic("provider cannot be nil") } + if disposeTTL <= 0 { + disposeTTL = defaultDisposeTTL + } + return &instanceManager{ - provider: provider, - cache: sync.Map{}, - locker: newLocker(), + provider: provider, + cache: sync.Map{}, + locker: newLocker(), + disposeTTL: disposeTTL, } } type instanceManager struct { - locker *locker - provider InstanceProvider - cache sync.Map + locker *locker + provider InstanceProvider + cache sync.Map + disposeTTL time.Duration + + disposeMutex sync.Mutex // Mutex to protect disposeTTL access in tests } func (im *instanceManager) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) { @@ -121,7 +134,7 @@ func (im *instanceManager) Get(ctx context.Context, pluginContext backend.Plugin } if disposer, valid := ci.instance.(InstanceDisposer); valid { - time.AfterFunc(disposeTTL, func() { + time.AfterFunc(im.disposeTTL, func() { disposer.Dispose() activeInstances.Dec() }) @@ -157,6 +170,14 @@ func (im *instanceManager) Do(ctx context.Context, pluginContext backend.PluginC return nil } +// setDisposeTTL sets the TTL for disposing instances. +// This method is only used for testing purposes to control the dispose timing behavior. +func (im *instanceManager) setDisposeTTL(ttl time.Duration) { + im.disposeMutex.Lock() + defer im.disposeMutex.Unlock() + im.disposeTTL = ttl +} + func callInstanceHandlerFunc(fn InstanceCallbackFunc, instance interface{}) { var params = []reflect.Value{} params = append(params, reflect.ValueOf(instance)) diff --git a/backend/instancemgmt/instance_manager_test.go b/backend/instancemgmt/instance_manager_test.go index 8351fe240..a0925de84 100644 --- a/backend/instancemgmt/instance_manager_test.go +++ b/backend/instancemgmt/instance_manager_test.go @@ -7,8 +7,9 @@ import ( "testing" "time" - "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/stretchr/testify/require" + + "github.com/grafana/grafana-plugin-sdk-go/backend" ) func TestInstanceManager(t *testing.T) { @@ -21,7 +22,7 @@ func TestInstanceManager(t *testing.T) { } tip := &testInstanceProvider{} - im := New(tip) + im := newInstanceManager(tip, time.Millisecond) t.Run("When getting instance should create a new instance", func(t *testing.T) { instance, err := im.Get(ctx, pCtx) @@ -43,11 +44,6 @@ func TestInstanceManager(t *testing.T) { Updated: time.Now(), }, } - origDisposeTTL := disposeTTL - disposeTTL = time.Millisecond - t.Cleanup(func() { - disposeTTL = origDisposeTTL - }) newInstance, err := im.Get(ctx, pCtxUpdated) t.Run("New instance should be created", func(t *testing.T) { @@ -110,12 +106,6 @@ func TestInstanceManagerConcurrency(t *testing.T) { }) t.Run("Check possible race condition issues when re-creating instance on settings update", func(t *testing.T) { - origDisposeTTL := disposeTTL - disposeTTL = time.Millisecond - t.Cleanup(func() { - disposeTTL = origDisposeTTL - }) - ctx := context.Background() initialPCtx := backend.PluginContext{ OrgID: 1, @@ -124,7 +114,8 @@ func TestInstanceManagerConcurrency(t *testing.T) { }, } tip := &testInstanceProvider{} - im := New(tip) + im := New(tip).(*instanceManager) + im.setDisposeTTL(time.Millisecond) // Creating initial instance with old contexts instanceToDispose, _ := im.Get(ctx, initialPCtx) diff --git a/backend/instancemgmt/intance_manager_ttl.go b/backend/instancemgmt/intance_manager_ttl.go new file mode 100644 index 000000000..ace2ffe6b --- /dev/null +++ b/backend/instancemgmt/intance_manager_ttl.go @@ -0,0 +1,122 @@ +package instancemgmt + +import ( + "context" + "fmt" + "time" + + gocache "github.com/patrickmn/go-cache" + + "github.com/grafana/grafana-plugin-sdk-go/backend" +) + +const ( + defaultInstanceTTL = 1 * time.Hour + defaultInstanceCleanupInterval = 2 * time.Hour +) + +// NewTTLInstanceManager creates a new instance manager with TTL-based caching. +// Instances will be automatically evicted from the cache after the specified TTL. +func NewTTLInstanceManager(provider InstanceProvider) InstanceManager { + return newTTLInstanceManager(provider, defaultInstanceTTL, defaultInstanceCleanupInterval) +} + +func newTTLInstanceManager(provider InstanceProvider, instanceTTL, instanceCleanupInterval time.Duration) InstanceManager { + if provider == nil { + panic("provider cannot be nil") + } + + // Use go-cache for TTL-based caching + cache := gocache.New(instanceTTL, instanceCleanupInterval) + + // Set up the OnEvicted callback to dispose instances + cache.OnEvicted(func(_ string, value interface{}) { + ci := value.(CachedInstance) + if disposer, valid := ci.instance.(InstanceDisposer); valid { + disposer.Dispose() + } + activeInstances.Dec() + }) + + return &instanceManagerWithTTL{ + provider: provider, + cache: cache, + locker: newLocker(), + } +} + +type instanceManagerWithTTL struct { + locker *locker + provider InstanceProvider + cache *gocache.Cache +} + +func (im *instanceManagerWithTTL) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) { + providerKey, err := im.provider.GetKey(ctx, pluginContext) + if err != nil { + return nil, err + } + // Double-checked locking for update/create criteria + cacheKey := fmt.Sprintf("%v", providerKey) + im.locker.RLock(cacheKey) + item, ok := im.cache.Get(cacheKey) + im.locker.RUnlock(cacheKey) + if ok { + ci := item.(CachedInstance) + needsUpdate := im.provider.NeedsUpdate(ctx, pluginContext, ci) + + if !needsUpdate { + im.locker.Lock(cacheKey) + im.refreshTTL(cacheKey, ci) + im.locker.Unlock(cacheKey) + return ci.instance, nil + } + } + + im.locker.Lock(cacheKey) + defer im.locker.Unlock(cacheKey) + + if item, ok := im.cache.Get(cacheKey); ok { + ci := item.(CachedInstance) + needsUpdate := im.provider.NeedsUpdate(ctx, pluginContext, ci) + + if !needsUpdate { + im.refreshTTL(cacheKey, ci) + return ci.instance, nil + } + + im.cache.Delete(cacheKey) + } + + instance, err := im.provider.NewInstance(ctx, pluginContext) + if err != nil { + return nil, err + } + im.cache.SetDefault(cacheKey, CachedInstance{ + PluginContext: pluginContext, + instance: instance, + }) + activeInstances.Inc() + + return instance, nil +} + +func (im *instanceManagerWithTTL) Do(ctx context.Context, pluginContext backend.PluginContext, fn InstanceCallbackFunc) error { + if fn == nil { + panic("fn cannot be nil") + } + + instance, err := im.Get(ctx, pluginContext) + if err != nil { + return err + } + + callInstanceHandlerFunc(fn, instance) + return nil +} + +// refreshTTL updates the TTL of the cached instance by resetting its expiration time. +func (im *instanceManagerWithTTL) refreshTTL(cacheKey string, ci CachedInstance) { + // SetDefault() technically creates a new cache entry with fresh TTL, effectively extending the instance's lifetime. + im.cache.SetDefault(cacheKey, ci) +} diff --git a/backend/instancemgmt/intance_manager_ttl_test.go b/backend/instancemgmt/intance_manager_ttl_test.go new file mode 100644 index 000000000..d75eb2893 --- /dev/null +++ b/backend/instancemgmt/intance_manager_ttl_test.go @@ -0,0 +1,332 @@ +package instancemgmt + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana-plugin-sdk-go/backend" +) + +func TestTTLInstanceManager(t *testing.T) { + ctx := context.Background() + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + + tip := &testInstanceProvider{} + im := NewTTLInstanceManager(tip) + + t.Run("When getting instance should create a new instance", func(t *testing.T) { + instance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, instance) + require.Equal(t, pCtx.OrgID, instance.(*testInstance).orgID) + require.Equal(t, pCtx.AppInstanceSettings.Updated, instance.(*testInstance).updated) + + t.Run("When getting instance should return same instance", func(t *testing.T) { + instance2, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.Same(t, instance, instance2) + }) + + t.Run("When updating plugin context and getting instance", func(t *testing.T) { + pCtxUpdated := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + + newInstance, err := im.Get(ctx, pCtxUpdated) + + t.Run("New instance should be created", func(t *testing.T) { + require.NoError(t, err) + require.NotNil(t, newInstance) + require.Equal(t, pCtxUpdated.OrgID, newInstance.(*testInstance).orgID) + require.Equal(t, pCtxUpdated.AppInstanceSettings.Updated, newInstance.(*testInstance).updated) + }) + + t.Run("New instance should not be the same as old instance", func(t *testing.T) { + require.NotSame(t, instance, newInstance) + }) + + t.Run("Old instance should be disposed", func(t *testing.T) { + instance.(*testInstance).wg.Wait() + require.True(t, instance.(*testInstance).disposed.Load()) + require.Equal(t, int64(1), instance.(*testInstance).disposedTimes.Load()) + }) + }) + }) +} + +func TestTTLInstanceManagerWithCustomTTL(t *testing.T) { + ctx := context.Background() + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + + tip := &testInstanceProvider{} + ttl := 10 * time.Millisecond + cleanupInterval := 5 * time.Millisecond + im := newTTLInstanceManager(tip, ttl, cleanupInterval) + + t.Run("Instance should be evicted after TTL", func(t *testing.T) { + if testing.Short() { + t.Skip("Tests with Sleep") + } + + instance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, instance) + + // Wait for TTL + cleanup interval + delta to ensure eviction + disposal + time.Sleep(ttl + cleanupInterval + 10*time.Millisecond) + + // Get instance again - should create a new one since old one was evicted + newInstance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, newInstance) + require.NotSame(t, instance, newInstance) + + // Original instance should be disposed after cleanup interval + instance.(*testInstance).wg.Wait() + require.True(t, instance.(*testInstance).disposed.Load()) + }) + + t.Run("Instance accessed before TTL expiry returns same instance", func(t *testing.T) { + if testing.Short() { + t.Skip("Tests with Sleep") + } + + instance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, instance) + + // Access instance before TTL expires + time.Sleep(ttl / 4) // Wait 2.5ms (well before 10ms expiry) + sameInstance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.Same(t, instance, sameInstance) + require.False(t, instance.(*testInstance).disposed.Load()) + }) + + t.Run("Instance accessed before TTL expiry should reset TTL", func(t *testing.T) { + if testing.Short() { + t.Skip("Tests with Sleep") + } + + instance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.NotNil(t, instance) + + // Wait 7ms (70% of 10ms TTL) - close to expiry but not expired yet + time.Sleep(7 * time.Millisecond) + + // Access instance before TTL expires - this should reset TTL + sameInstance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.Same(t, instance, sameInstance) + + // Now wait 5ms more (total 12ms from instance creation) + // Original TTL would have expired at 10ms, but reset TTL should be (7ms)+10ms = 17ms + // So at 12ms, we should still have the same instance + time.Sleep(5 * time.Millisecond) + + stillSameInstance, err := im.Get(ctx, pCtx) + require.NoError(t, err) + require.Same(t, instance, stillSameInstance) + require.False(t, instance.(*testInstance).disposed.Load()) + }) +} + +func TestTTLInstanceManagerConcurrency(t *testing.T) { + t.Run("Check possible race condition issues when initially creating instance", func(t *testing.T) { + ctx := context.Background() + tip := &testInstanceProvider{} + im := NewTTLInstanceManager(tip) + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + var wg sync.WaitGroup + wg.Add(10) + + var createdInstances []*testInstance + mutex := new(sync.Mutex) + // Creating new instances concurrently + for i := 0; i < 10; i++ { + go func() { + instance, _ := im.Get(ctx, pCtx) + mutex.Lock() + defer mutex.Unlock() + // Collect all instances created + createdInstances = append(createdInstances, instance.(*testInstance)) + wg.Done() + }() + } + wg.Wait() + + t.Run("All concurrent gets should return the same instance", func(t *testing.T) { + // All instances should be the same (no race condition) + firstInstance := createdInstances[0] + for _, instance := range createdInstances { + require.Same(t, firstInstance, instance) + } + }) + }) + + t.Run("Check possible race condition issues when re-creating instance on settings update", func(t *testing.T) { + ctx := context.Background() + initialPCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + tip := &testInstanceProvider{} + im := NewTTLInstanceManager(tip) + // Creating initial instance with old contexts + instanceToDispose, _ := im.Get(ctx, initialPCtx) + + updatedPCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + + var wg sync.WaitGroup + wg.Add(10) + + var createdInstances []*testInstance + mutex := new(sync.Mutex) + // Creating new instances because of updated context + for i := 0; i < 10; i++ { + go func() { + instance, _ := im.Get(ctx, updatedPCtx) + mutex.Lock() + defer mutex.Unlock() + // Collect all instances created during concurrent update + createdInstances = append(createdInstances, instance.(*testInstance)) + wg.Done() + }() + } + wg.Wait() + + t.Run("Initial instance should be disposed only once", func(t *testing.T) { + instanceToDispose.(*testInstance).wg.Wait() + require.Equal(t, int64(1), instanceToDispose.(*testInstance).disposedTimes.Load()) + }) + t.Run("All created instances should be the same (no race condition)", func(t *testing.T) { + // All new instances should be the same + if len(createdInstances) > 0 { + firstInstance := createdInstances[0] + for _, instance := range createdInstances { + require.Same(t, firstInstance, instance) + } + } + }) + }) + + t.Run("Long recreation of instance should not affect other instances", func(t *testing.T) { + const delay = time.Millisecond * 50 + ctx := context.Background() + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + if testing.Short() { + t.Skip("Tests with Sleep") + } + + tip := &testInstanceProvider{delay: delay} + im := NewTTLInstanceManager(tip) + // Creating instance with id#1 in cache + _, err := im.Get(ctx, pCtx) + require.NoError(t, err) + var wg1, wg2 sync.WaitGroup + wg1.Add(1) + wg2.Add(1) + go func() { + // Creating instance with id#2 in cache + wg1.Done() + _, err := im.Get(ctx, backend.PluginContext{ + OrgID: 2, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + }) + require.NoError(t, err) + wg2.Done() + }() + // Waiting before thread 2 starts to get the instance, so thread 2 could acquire the lock before thread 1 + wg1.Wait() + // Getting existing instance with id#1 from cache + start := time.Now() + _, err = im.Get(ctx, pCtx) + elapsed := time.Since(start) + require.NoError(t, err) + // Waiting before thread 2 finished to get the instance + wg2.Wait() + if elapsed > delay { + require.Fail(t, "Instance should be retrieved from cache without delay") + } + }) +} + +func TestTTLInstanceManagerDo(t *testing.T) { + ctx := context.Background() + pCtx := backend.PluginContext{ + OrgID: 1, + AppInstanceSettings: &backend.AppInstanceSettings{ + Updated: time.Now(), + }, + } + + tip := &testInstanceProvider{} + im := NewTTLInstanceManager(tip) + + t.Run("Do should execute callback with instance", func(t *testing.T) { + var callbackInstance Instance + err := im.Do(ctx, pCtx, func(instance Instance) { + callbackInstance = instance + }) + require.NoError(t, err) + require.NotNil(t, callbackInstance) + require.Equal(t, pCtx.OrgID, callbackInstance.(*testInstance).orgID) + }) + + t.Run("Do should panic with nil callback", func(t *testing.T) { + require.Panics(t, func() { + _ = im.Do(ctx, pCtx, nil) + }) + }) +} + +func TestTTLInstanceManagerPanicHandling(t *testing.T) { + t.Run("NewTTLInstanceManager should panic with nil provider", func(t *testing.T) { + require.Panics(t, func() { + NewTTLInstanceManager(nil) + }) + }) + + t.Run("newTTLInstanceManager should panic with nil provider", func(t *testing.T) { + require.Panics(t, func() { + newTTLInstanceManager(nil, defaultInstanceTTL, defaultInstanceCleanupInterval) + }) + }) +} diff --git a/experimental/featuretoggles/featuretoggles.go b/experimental/featuretoggles/featuretoggles.go index 907fda18d..58646e251 100644 --- a/experimental/featuretoggles/featuretoggles.go +++ b/experimental/featuretoggles/featuretoggles.go @@ -7,6 +7,10 @@ import ( const ( EnabledFeatures = "GF_INSTANCE_FEATURE_TOGGLES_ENABLE" + + // TTLInstanceManager is a feature toggle for enabling the TTL-based instance manager. + // When enabled, instances will be automatically evicted from the cache after a configurable TTL. + TTLInstanceManager = "ttlPluginInstanceManager" ) // FeatureToggles can check if feature toggles are enabled on the Grafana instance. diff --git a/go.mod b/go.mod index 0fcfe8d69..29db59865 100644 --- a/go.mod +++ b/go.mod @@ -98,6 +98,7 @@ require ( github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect github.com/oklog/run v1.0.0 // indirect github.com/oklog/ulid v1.3.1 // indirect + github.com/patrickmn/go-cache v2.1.0+incompatible github.com/perimeterx/marshmallow v1.1.5 // indirect github.com/pierrec/lz4/v4 v4.1.22 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect diff --git a/go.sum b/go.sum index 6d4ec823e..92a9ce097 100644 --- a/go.sum +++ b/go.sum @@ -184,6 +184,8 @@ github.com/oklog/ulid v1.3.1 h1:EGfNDEx6MqHz8B3uNV6QAib1UR2Lm97sHi3ocA6ESJ4= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= +github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc= +github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s= github.com/perimeterx/marshmallow v1.1.5/go.mod h1:dsXbUu8CRzfYP5a87xpp0xq9S3u0Vchtcl8we9tYaXw= github.com/pierrec/lz4/v4 v4.1.22 h1:cKFw6uJDK+/gfw5BcDL0JL5aBsAFdsIT18eRtLj7VIU=