Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions router/core/graph_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,8 @@ type graphMux struct {
mux *chi.Mux
reused atomic.Bool

planCacheHookEnabled atomic.Bool

planCache *ristretto.Cache[uint64, *planWithMetaData]
planFallbackCache *slowplancache.Cache[*planWithMetaData]
persistedOperationCache *ristretto.Cache[uint64, NormalizationCacheEntry]
Expand Down Expand Up @@ -721,10 +723,14 @@ func (s *graphMux) buildOperationCaches(srv *graphServer) (computeSha256 bool, e
BufferItems: 64,
}
if srv.cacheWarmup != nil && srv.cacheWarmup.Enabled && srv.cacheWarmup.InMemoryFallback {
s.planCacheHookEnabled.Store(true)
planCacheConfig.OnEvict = func(item *ristretto.Item[*planWithMetaData]) {
// This could be called before planFallbackCache is set, but it's not a problem
// because there is a nil guard inside, as well as items should not really be evicted
// on startup
if !s.planCacheHookEnabled.Load() {
return
}
s.planFallbackCache.Set(item.Key, item.Value, item.Value.planningDuration)
}
}
Expand Down Expand Up @@ -961,7 +967,11 @@ func (s *graphMux) Shutdown(ctx context.Context) error {
// cancel the graph muxes context to close its resources like websocket connections, resolvers, etc.
s.cancel()

// Bypass ristretto OnEvict hook, to avoid it migrating plan caches into slowplancache.
// This is not meant to run on graph mux shutdown.
s.planCacheHookEnabled.Store(false)
s.planCache.Close()

s.planFallbackCache.Close()
s.persistedOperationCache.Close()
s.normalizationCache.Close()
Expand Down
15 changes: 7 additions & 8 deletions router/core/operation_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ import (
)

type planWithMetaData struct {
preparedPlan plan.Plan
operationDocument, schemaDocument *ast.Document
typeFieldUsageInfo []*graphqlschemausage.TypeFieldUsageInfo
argumentUsageInfo []*graphqlmetricsv1.ArgumentUsageInfo
content string
operationName string
planningDuration time.Duration
preparedPlan plan.Plan
operationDocument *ast.Document
typeFieldUsageInfo []*graphqlschemausage.TypeFieldUsageInfo
argumentUsageInfo []*graphqlmetricsv1.ArgumentUsageInfo
content string
operationName string
planningDuration time.Duration
}

type OperationPlanner struct {
Expand Down Expand Up @@ -96,7 +96,6 @@ func (p *OperationPlanner) planOperation(content string, name string, includeQue
return &planWithMetaData{
preparedPlan: preparedPlan,
operationDocument: &doc,
schemaDocument: p.executor.RouterSchema,
}, nil
}

Expand Down
12 changes: 12 additions & 0 deletions router/pkg/slowplancache/slow_plan_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,17 @@ func (c *Cache[V]) Close() {
// This downside is also there in ristretto (if set is called concurrently)
// it is even documented in the ristretto code as a comment
close(c.writeCh)

// Drop cached entries so Close releases references to stored values
// (e.g. query plans holding schema AST pointers) before the Cache
// struct is GC'd.
c.entries.Range(func(key, _ any) bool {
c.entries.Delete(key)
return true
})

c.size = 0
c.minKey = 0
c.minDur = 0
})
}
21 changes: 21 additions & 0 deletions router/pkg/slowplancache/slow_plan_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,27 @@ func TestCache_DoubleClose(t *testing.T) {
})
}

func TestCache_CloseReleasesEntries(t *testing.T) {
t.Parallel()
c, err := New[*testPlan](10, 0)
require.NoError(t, err)

c.Set(1, &testPlan{content: "q1"}, 10*time.Millisecond)
c.Set(2, &testPlan{content: "q2"}, 20*time.Millisecond)
c.Set(3, &testPlan{content: "q3"}, 30*time.Millisecond)
c.Wait()

c.Close()

count := 0
c.entries.Range(func(_, _ any) bool {
count++
return true
})
require.Equal(t, 0, count, "entries sync.Map must be empty after Close")
require.Equal(t, int64(0), c.size)
}

func BenchmarkCache_Set(b *testing.B) {
c, err := New[*testPlan](1000, 0)
require.NoError(b, err)
Expand Down
Loading