Skip to content
Draft
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
11 changes: 11 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

planCacheOnEvictEnabled 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.planCacheOnEvictEnabled.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.planCacheOnEvictEnabled.Load() {
return
}
s.planFallbackCache.Set(item.Key, item.Value, item.Value.planningDuration)
}
}
Expand Down Expand Up @@ -961,6 +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()

// ristretto Close() invokes OnEvict for every entry; skip slow-cache migration during shutdown.
s.planCacheOnEvictEnabled.Store(false)
if s.planFallbackCache != nil {
s.planFallbackCache.Wait()
}
s.planCache.Close()
s.planFallbackCache.Close()
s.persistedOperationCache.Close()
Expand Down
5 changes: 2 additions & 3 deletions router/core/operation_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
)

type planWithMetaData struct {
preparedPlan plan.Plan
operationDocument, schemaDocument *ast.Document
preparedPlan plan.Plan
operationDocument *ast.Document
typeFieldUsageInfo []*graphqlschemausage.TypeFieldUsageInfo
argumentUsageInfo []*graphqlmetricsv1.ArgumentUsageInfo
content string
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
4 changes: 4 additions & 0 deletions router/core/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,10 @@ func (r *Router) serverTLSConfig() (*tls.Config, error) {

// newGraphServer creates a new server.
func (r *Router) newServer(ctx context.Context, response *routerconfig.Response) error {
// Extract slow-plan cache entries before building the new graph server, which
// overwrites ReloadPersistentState cache references and before the old graphMux shuts down.
r.reloadPersistentState.OnRouterConfigReload()

server, err := newGraphServer(ctx, r, response, r.proxy)
if err != nil {
r.logger.Error("Failed to create graph server. Keeping the old server", zap.Error(err))
Expand Down
9 changes: 9 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,14 @@ 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
})
}
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