Websocket test - #21
Draft
damian-rakus wants to merge 37 commits into
Draft
Conversation
Allows custom modules to mark a request as having a wildcard scope that satisfies all @requiresScopes checks. Authentication is still enforced. Closes wundergraph#2490 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace require.Contains with exact require.Equal assertions - Add test for RejectOperationIfUnauthorized + wildcard scope - Clarify doc comment: authentication is a prerequisite for wildcard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move wildcardScopeKey type to authorizer.go next to hasWildcardScope - Replace raw JSON string assertions with structured graphQLResponse type for readable test assertions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eak (#2) * fix(router): clear slowplancache entries on Close to prevent memory leak During config reloads, ristretto's OnEvict callback pushes plan entries into the slowplancache. Each entry holds a pointer to the schema AST document (~200MB). When slowplancache.Close() is called, it stops the background goroutine but never clears the sync.Map entries, keeping old schemas pinned in memory until the entire Cache struct is GC'd — which may be delayed by goroutines still referencing the owning graphMux. Clear all entries in Close() so that references to expensive objects (like *ast.Document) are released immediately. * fix(router): guard slowplancache Close() clearing with mondaytweaks flag Add router/pkg/mondaytweaks package (same pattern as graphql-go-tools) for compile-time feature flags. Guard the entry-clearing fix behind mondaytweaks.ClearSlowPlanCacheOnClose so it's easy to upstream later.
* fix(router): release schema refs on config reload to prevent memory leak Stop storing schemaDocument in cached planWithMetaData entries so plan caches no longer pin the old router schema AST (~200MB) after CDN reloads. Also call OnRouterConfigReload before building a new graph server so slow-plan cache entries are extracted while the old graphMux is still referenced, matching the supervisor restart path. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): drain WS subs and skip plan cache OnEvict on mux shutdown Disable ristretto OnEvict migration into slowplancache when a graphMux is shutting down, since Close() clears every entry and the fallback cache is about to be closed anyway. Close websocket subscriptions synchronously before plan caches so preparedPlan and executor refs are released first. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): register heap pprof handlers for in-use profiling Expose /debug/pprof/heap and related routes on the pprof server so forced-GC heap snapshots (heap?gc=1) work for memory leak diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): re-read PPROF_ADDR from env after flag.Parse Flag defaults are captured at package init before embedders set PPROF_ADDR in main(), so platform-api ensurePprofAddr had no effect. Re-read env after flag.Parse() matches the existing CONFIG_PATH pattern. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): release executor schema refs and reduce upstream WS overhead on reload Executor.Close() nils federation schema AST held after graph mux shutdown. Share one upstream subscription client across subgraph factories and disable upstream ping loops when client WebSocket is disabled. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): use noop upstream subscription client when subscriptions unused Skip WSTransport/SSE initialization when the router schema has no subscription root fields or when client WebSocket and pubsub events are both disabled. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): nil graphMux caches after shutdown to allow GC on reload Close and drop Ristretto cache pointers, wsHandler, and mux after graphMux shutdown, and remove shut-down muxes from graphMuxList. Local benchmark: ~74 MB/reload → ~12 MB/reload retained inuse (same-content manifest reloads). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): skip unchanged manifest reloads and release stale execution config Hash mapper.json before re-assembling; skip reload when content is unchanged (mtime-only touches). After a successful reload, swap staticExecutionConfig and proto.Reset the previous config so decoded protojson strings can be collected. * fix(router): reuse graph muxes on manifest reload and release shutdown refs Pass Changes/Hashes from mapper.json graph hashes on the manifest watcher path so unchanged base or feature-flag muxes survive config reloads. Nil graphServer and graphMux metric fields after shutdown to drop retained references sooner. * Revert "fix(router): reuse graph muxes on manifest reload and release shutdown refs" This reverts commit 2c32b45. * chore(router): drop profiling/pyroscope changes from PR Remove late env re-read for PPROF/PYROSCOPE in main.go and extra pprof handlers in profile.go so this PR stays focused on config reload fixes. * fix(router): gate memory-leak fixes behind mondaytweaks constants Centralize all monday.com config-reload leak fixes in mondaytweaks.go so they are easy to audit and disable individually. Restore profiling helpers from stash behind separate tweak flags. * chore(router): drop profiling and pyroscope mondaytweaks Remove PPROF/PYROSCOPE env re-read, heap pprof routes, and Pyroscope name/tag helpers so the PR stays focused on config reload memory fixes. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…flag) (#7) * perf(router): size-aware execution-plan cache eviction (mondaytweaks flag) The execution-plan Ristretto cache evicts by entry count (every entry costs 1, MaxCost = ExecutionPlanCacheSize), so a single structurally-unique aliased-batch mutation plan occupies one slot regardless of its true retained size. On US cluster group 02 a burst of such giant plans could evict thousands of small hot plans while collectively pinning most of the plan-cache heap — the dominant driver of the 02 vs 01 RSS gap (plan cache ~16% of heap on 02 vs ~4% on 01). Adds mondaytweaks.SizeAwarePlanCache (default off, canary-first): when enabled, MaxCost becomes ExecutionPlanCacheSize * PlanCacheSizeAwareBudgetPerSlotBytes (a byte budget) and each entry is charged estimatePlanCacheCost — a cheap O(number-of-slices) estimate of the retained heap keyed off operationDocument (always populated) plus the raw operation bytes. Giant plans then cost tens of slots and evict first, and total plan-cache heap is bounded to a predictable ceiling. NumCounters stays keyed to the expected entry count for TinyLFU. Default off so existing count-based behaviour (and the plan-fallback-cache tests that rely on ExecutionPlanCacheSize=1 forcing single-entry eviction) is unchanged; intended to be enabled as a per-cluster canary starting with 02. * perf(router): enable size-aware plan cache by default with per-instance opt-out Flip mondaytweaks.SizeAwarePlanCache on by default so the execution-plan cache evicts by estimated retained heap (byte budget) instead of entry count. Defaulting it on would break the plan-fallback-cache integration tests, which rely on count-based single-entry eviction (ExecutionPlanCacheSize=1) to force eviction and exercise the fallback trigger. Since router-tests run with -race, toggling the global flag inside those parallel tests is not safe. Instead add a per-instance opt-out, config.EngineExecutionConfiguration. DisableSizeAwarePlanCache (programmatic only, no env/yaml binding). Both the cache-budget computation (graph_server) and the plan-cache Set cost (operation_planner) now consult sizeAwarePlanCacheEnabled(cfg) so cost and MaxCost always agree, and the fallback tests set DisableSizeAwarePlanCache=true to pin deterministic count-based eviction without mutating the global. Production leaves the override false and follows the mondaytweaks default (on).
… to fix shutdown data race (#8)
…undergraph#3035 Local reload benchmarks showed ~7.6 MB/reload retained with flags off vs pre-fix ~260 MB, so the flag-guarded reload cleanup is redundant with upstream graphMux closure fixes. Drop the eight memory-leak toggles and their guarded code while keeping behavior/perf tweaks and the structural schemaDocument removal in cached plans.
Turn off the three subscription/websocket-disablement compile-time flags, reverting to upstream default subscription-client behavior: - ShareUpstreamSubscriptionClient - UseNoopUpstreamSubscriptionClientWhenUnused - DisableUpstreamSubscriptionPingWhenClientWebSocketDisabled Suspected of interfering with CDN config hot reload. Leaves perf/observability tweaks (SizeAwarePlanCache, fetch-count field) untouched.
…12) SwapGraphServer shuts the previous graph server down synchronously on the config-poller goroutine with an unbounded ctx. graphServer.Shutdown drains in-flight requests via wait() (polls inFlightRequests, no independent timeout), so a single stuck request — e.g. a handler blocked on a dead-client write (WriteTimeout=0) or a custom module's own HTTP call — freezes the entire config pipeline. Production showed in-flight-drain stalls up to 1h21m, which: - froze CDN config hot-reload → pods served stale schema for hours (#3286) - pinned the old generation's schema AST + ristretto caches + protojson in memory, driving a GC mark-phase storm and planning-latency spikes. Behind mondaytweaks.AsyncBoundedOldGraphServerShutdown (default on): run the old server's Shutdown off the poller goroutine and bound the drain by the configured grace_period (fallback 90s > 60s subgraph request_timeout when unset), via context.WithoutCancel + WithTimeout. New traffic already routes to the swapped-in server, so abandoning a stuck old-server request after the drain window is safe and lets reloads proceed + releases the old generation. Also re-enable DisableUpstreamSubscriptionPingWhenClientWebSocketDisabled: prod runs websocket.enabled=false, yet a goroutine profile showed WSTransport.pingLoop at ~65% of all goroutines (1.5M) accumulating across reloads. Zeroing PingInterval when client WS is disabled stops that leak.
…eFieldDependencies flag (#18)
…estimator (#19) * fix(router): count fetch tree and response fields in plan cache cost estimator * test(router): cover fetch tree and response field counting in plan cache cost * feat(router): gate plan cache tree-walk cost behind PlanCacheCostCountsPlanTree flag * feat(mondaytweaks): gate graphql Source caching behind ReuseGraphQLSource flag * chore(router): replace graphql-go-tools with local pool-graphql-source worktree * Revert "chore(router): replace graphql-go-tools with local pool-graphql-source worktree" This reverts commit e940ef7. * fix(mondaytweaks): move ReuseGraphQLSource flag to graphql-go-tools package
…ndedOldGraphServerShutdown, useNoopUpstreamSubscriptionClientWhenUnused flags All three were disabled (false) or reverted to upstream defaults due to suspected interference with CDN config hot-reload. Remove them along with their supporting infrastructure: noop subscription client files, WebSocketConfiguration field in ExecutorBuildOptions, gracePeriod machinery in http_server, and the sharedSubscriptionClient path in factoryresolver.
- Inline subscriptionClientForFactory (trivial wrapper after ShareUpstreamSubscriptionClient removal) - Revert NewWebsocketMiddleware second return value (*WebsocketHandler unused) - Revert demo.config.yaml to upstream (local dev additions)
This reverts commit 2bedc2a.
…form The newSubscriptionClient() extraction was introduced to support the shared/noop subscription client paths, both of which have been removed. Inline the construction back to match upstream exactly.
…ment planFallbackCache is conditionally created and slowplancache.Set has a nil receiver guard, but that's non-obvious. The comment explaining why the call is safe was dropped when SkipPlanCacheOnEvictDuringMuxShutdown was removed; restore it.
The sync-handler tracking and ShutdownConnections were already removed in 9455ffe. Remove the leftover closeOnce field and Do wrapper to fully revert websocket.go to upstream.
aws-lambda-router - uncommitted changes detectedSeems like you forgot to commit some code. Possible causes:
Dirty files
|
connect-go - uncommitted changes detectedSeems like you forgot to commit some code. Possible causes:
Dirty files
|
Router-nonroot image scan failed❌ Security vulnerabilities found in image: Please check the security vulnerabilities found in the PR. If you believe this is a false positive, please add the vulnerability to the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.