Skip to content

Commit 9fdee90

Browse files
Rm client proxy cache flag (#2209)
1 parent ac189cc commit 9fdee90

File tree

6 files changed

+91
-81
lines changed

6 files changed

+91
-81
lines changed

packages/api/internal/orchestrator/orchestrator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func New(
112112

113113
var routingCatalog e2bcatalog.SandboxesCatalog
114114
if redisClient != nil {
115-
routingCatalog = e2bcatalog.NewRedisSandboxesCatalog(redisClient, featureFlags)
115+
routingCatalog = e2bcatalog.NewRedisSandboxCatalog(redisClient, e2bcatalog.NewReadThroughSandboxCache())
116116
} else {
117117
routingCatalog = e2bcatalog.NewMemorySandboxesCatalog()
118118
}

packages/client-proxy/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func run() int {
120120
l.Error(ctx, "Failed to close redis client", zap.Error(err))
121121
}
122122
}()
123-
catalog = e2bcatalog.NewRedisSandboxesCatalog(redisClient, featureFlagsClient)
123+
catalog = e2bcatalog.NewRedisSandboxCatalog(redisClient, e2bcatalog.NewNoopSandboxCache())
124124
} else {
125125
if !errors.Is(err, factories.ErrRedisDisabled) {
126126
l.Error(ctx, "Failed to create redis client", zap.Error(err))

packages/shared/pkg/featureflags/flags.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ var (
108108
EdgeProvidedSandboxMetricsFlag = newBoolFlag("edge-provided-sandbox-metrics", false)
109109
CreateStorageCacheSpansFlag = newBoolFlag("create-storage-cache-spans", env.IsDevelopment())
110110
SandboxAutoResumeFlag = newBoolFlag("sandbox-auto-resume", env.IsDevelopment())
111-
SandboxCatalogLocalCacheFlag = newBoolFlag("sandbox-catalog-local-cache", true)
112111

113112
// PeerToPeerChunkTransferFlag enables peer-to-peer chunk routing.
114113
PeerToPeerChunkTransferFlag = newBoolFlag("peer-to-peer-chunk-transfer", false)
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package sandbox_catalog
2+
3+
import (
4+
"time"
5+
6+
"github.com/jellydator/ttlcache/v3"
7+
)
8+
9+
type NoopSandboxCache struct{}
10+
11+
var _ SandboxCache = (*NoopSandboxCache)(nil)
12+
13+
func NewNoopSandboxCache() *NoopSandboxCache {
14+
return &NoopSandboxCache{}
15+
}
16+
17+
func (n *NoopSandboxCache) Get(string, ...ttlcache.Option[string, *SandboxInfo]) *ttlcache.Item[string, *SandboxInfo] {
18+
return nil
19+
}
20+
21+
func (n *NoopSandboxCache) Set(string, *SandboxInfo, time.Duration) *ttlcache.Item[string, *SandboxInfo] {
22+
return nil
23+
}
24+
25+
func (n *NoopSandboxCache) Delete(string) {}
26+
27+
func (n *NoopSandboxCache) Stop() {}

packages/shared/pkg/sandbox-catalog/catalog_redis.go

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/redis/go-redis/v9"
1212
"go.uber.org/zap"
1313

14-
"github.com/e2b-dev/infra/packages/shared/pkg/featureflags"
1514
"github.com/e2b-dev/infra/packages/shared/pkg/logger"
1615
)
1716

@@ -23,38 +22,44 @@ const (
2322
catalogRedisLocalCacheTtl = time.Millisecond * 500
2423
)
2524

25+
type SandboxCache interface {
26+
Get(key string, opts ...ttlcache.Option[string, *SandboxInfo]) *ttlcache.Item[string, *SandboxInfo]
27+
Set(key string, value *SandboxInfo, ttl time.Duration) *ttlcache.Item[string, *SandboxInfo]
28+
Delete(key string)
29+
Stop()
30+
}
31+
2632
type RedisSandboxCatalog struct {
27-
redisClient redis.UniversalClient
28-
cache *ttlcache.Cache[string, *SandboxInfo]
29-
featureFlags *featureflags.Client
33+
redisClient redis.UniversalClient
34+
cache SandboxCache
3035
}
3136

3237
var _ SandboxesCatalog = (*RedisSandboxCatalog)(nil)
3338

34-
func NewRedisSandboxesCatalog(redisClient redis.UniversalClient, featureFlags *featureflags.Client) *RedisSandboxCatalog {
35-
cache := ttlcache.New(ttlcache.WithTTL[string, *SandboxInfo](catalogRedisLocalCacheTtl), ttlcache.WithDisableTouchOnHit[string, *SandboxInfo]())
39+
func NewReadThroughSandboxCache() *ttlcache.Cache[string, *SandboxInfo] {
40+
cache := ttlcache.New(
41+
ttlcache.WithTTL[string, *SandboxInfo](catalogRedisLocalCacheTtl),
42+
ttlcache.WithDisableTouchOnHit[string, *SandboxInfo](),
43+
)
3644
go cache.Start()
3745

46+
return cache
47+
}
48+
49+
func NewRedisSandboxCatalog(redisClient redis.UniversalClient, cache SandboxCache) *RedisSandboxCatalog {
3850
return &RedisSandboxCatalog{
39-
redisClient: redisClient,
40-
cache: cache,
41-
featureFlags: featureFlags,
51+
redisClient: redisClient,
52+
cache: cache,
4253
}
4354
}
4455

45-
var _ SandboxesCatalog = (*RedisSandboxCatalog)(nil)
46-
4756
func (c *RedisSandboxCatalog) GetSandbox(ctx context.Context, sandboxID string) (*SandboxInfo, error) {
4857
spanCtx, span := tracer.Start(ctx, "sandbox-catalog-get")
4958
defer span.End()
5059

51-
useLocalCache := c.featureFlags.BoolFlag(spanCtx, featureflags.SandboxCatalogLocalCacheFlag)
52-
53-
if useLocalCache {
54-
sandboxInfo := c.cache.Get(sandboxID)
55-
if sandboxInfo != nil {
56-
return sandboxInfo.Value(), nil
57-
}
60+
sandboxInfo := c.cache.Get(sandboxID)
61+
if sandboxInfo != nil {
62+
return sandboxInfo.Value(), nil
5863
}
5964

6065
ctx, ctxCancel := context.WithTimeout(spanCtx, catalogRedisTimeout)
@@ -75,9 +80,7 @@ func (c *RedisSandboxCatalog) GetSandbox(ctx context.Context, sandboxID string)
7580
return nil, fmt.Errorf("failed to unmarshal sandbox info: %w", err)
7681
}
7782

78-
if useLocalCache {
79-
c.cache.Set(sandboxID, info, catalogRedisLocalCacheTtl)
80-
}
83+
c.cache.Set(sandboxID, info, catalogRedisLocalCacheTtl)
8184

8285
return info, nil
8386
}
@@ -101,9 +104,7 @@ func (c *RedisSandboxCatalog) StoreSandbox(ctx context.Context, sandboxID string
101104
return fmt.Errorf("failed to store sandbox info in redis: %w", status.Err())
102105
}
103106

104-
if c.featureFlags.BoolFlag(spanCtx, featureflags.SandboxCatalogLocalCacheFlag) {
105-
c.cache.Set(sandboxID, sandboxInfo, catalogRedisLocalCacheTtl)
106-
}
107+
c.cache.Set(sandboxID, sandboxInfo, catalogRedisLocalCacheTtl)
107108

108109
return nil
109110
}

packages/shared/pkg/sandbox-catalog/catalog_redis_test.go

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,20 @@ import (
66
"testing"
77
"time"
88

9-
"github.com/launchdarkly/go-server-sdk/v7/testhelpers/ldtestdata"
9+
"github.com/redis/go-redis/v9"
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
1212

13-
"github.com/e2b-dev/infra/packages/shared/pkg/featureflags"
1413
redis_utils "github.com/e2b-dev/infra/packages/shared/pkg/redis"
1514
)
1615

17-
func TestRedisCatalog_LocalCacheFlagServiceContext(t *testing.T) {
18-
t.Parallel()
16+
func seedRedisCatalogEntry(t *testing.T) (context.Context, redis.UniversalClient, string, *SandboxInfo) {
17+
t.Helper()
1918

2019
ctx := t.Context()
2120
redisClient := redis_utils.SetupInstance(t)
2221

23-
source := ldtestdata.DataSource()
24-
source.Update(
25-
source.Flag(featureflags.SandboxCatalogLocalCacheFlag.Key()).
26-
VariationForAll(true).
27-
VariationForKey(featureflags.ServiceKind, "client-proxy", false),
28-
)
29-
30-
sbxID := "sbx-service-context"
22+
sbxID := "sbx-local-cache"
3123
expected := &SandboxInfo{
3224
OrchestratorID: "orch-1",
3325
OrchestratorIP: "10.0.0.1",
@@ -40,52 +32,43 @@ func TestRedisCatalog_LocalCacheFlagServiceContext(t *testing.T) {
4032
require.NoError(t, err)
4133
require.NoError(t, redisClient.Set(ctx, "sandbox:catalog:"+sbxID, data, time.Minute).Err())
4234

43-
t.Run("service-targeted disable prevents local cache write", func(t *testing.T) {
44-
t.Parallel()
45-
ctx := t.Context()
46-
47-
ff, err := featureflags.NewClientWithDatasource(source)
48-
require.NoError(t, err)
49-
ff.SetServiceName("client-proxy")
50-
t.Cleanup(func() {
51-
assert.NoError(t, ff.Close(context.Background()))
52-
})
35+
return ctx, redisClient, sbxID, expected
36+
}
5337

54-
catalog := NewRedisSandboxesCatalog(redisClient, ff)
55-
t.Cleanup(func() {
56-
assert.NoError(t, catalog.Close(context.Background()))
57-
})
38+
func TestRedisCatalog_NoopCacheRemainsEmpty(t *testing.T) {
39+
t.Parallel()
5840

59-
got, err := catalog.GetSandbox(ctx, sbxID)
60-
require.NoError(t, err)
61-
require.Equal(t, expected.OrchestratorID, got.OrchestratorID)
62-
require.Equal(t, expected.ExecutionID, got.ExecutionID)
41+
ctx, redisClient, sbxID, expected := seedRedisCatalogEntry(t)
6342

64-
assert.Nil(t, catalog.cache.Get(sbxID), "local cache should remain empty when flag is disabled for this service")
43+
cache := NewNoopSandboxCache()
44+
noCacheCatalog := NewRedisSandboxCatalog(redisClient, cache)
45+
t.Cleanup(func() {
46+
assert.NoError(t, noCacheCatalog.Close(context.Background()))
6547
})
6648

67-
t.Run("other service uses local cache", func(t *testing.T) {
68-
t.Parallel()
69-
ctx := t.Context()
70-
71-
ff, err := featureflags.NewClientWithDatasource(source)
72-
require.NoError(t, err)
73-
ff.SetServiceName("orchestration-api")
74-
t.Cleanup(func() {
75-
assert.NoError(t, ff.Close(context.Background()))
76-
})
77-
78-
catalog := NewRedisSandboxesCatalog(redisClient, ff)
79-
t.Cleanup(func() {
80-
assert.NoError(t, catalog.Close(context.Background()))
81-
})
82-
83-
got, err := catalog.GetSandbox(ctx, sbxID)
84-
require.NoError(t, err)
85-
require.Equal(t, expected.OrchestratorIP, got.OrchestratorIP)
86-
87-
item := catalog.cache.Get(sbxID)
88-
require.NotNil(t, item, "local cache should be populated when flag is enabled")
89-
require.Equal(t, expected.ExecutionID, item.Value().ExecutionID)
49+
got, err := noCacheCatalog.GetSandbox(ctx, sbxID)
50+
require.NoError(t, err)
51+
require.Equal(t, expected.OrchestratorID, got.OrchestratorID)
52+
require.Equal(t, expected.ExecutionID, got.ExecutionID)
53+
assert.Nil(t, cache.Get(sbxID))
54+
}
55+
56+
func TestRedisCatalog_ReadThroughCachePopulatesLocalCache(t *testing.T) {
57+
t.Parallel()
58+
59+
ctx, redisClient, sbxID, expected := seedRedisCatalogEntry(t)
60+
61+
cache := NewReadThroughSandboxCache()
62+
cachedCatalog := NewRedisSandboxCatalog(redisClient, cache)
63+
t.Cleanup(func() {
64+
assert.NoError(t, cachedCatalog.Close(context.Background()))
9065
})
66+
67+
got, err := cachedCatalog.GetSandbox(ctx, sbxID)
68+
require.NoError(t, err)
69+
require.Equal(t, expected.OrchestratorIP, got.OrchestratorIP)
70+
71+
item := cache.Get(sbxID)
72+
require.NotNil(t, item, "local cache should be populated when enabled")
73+
require.Equal(t, expected.ExecutionID, item.Value().ExecutionID)
9174
}

0 commit comments

Comments
 (0)