Skip to content

fix: stop leaking cached envoy snapshots - #11309

Open
chandler-solo wants to merge 12 commits into
mainfrom
chandler/clearsnapshot
Open

fix: stop leaking cached envoy snapshots#11309
chandler-solo wants to merge 12 commits into
mainfrom
chandler/clearsnapshot

Conversation

@chandler-solo

@chandler-solo chandler-solo commented Jun 30, 2026

Copy link
Copy Markdown

Description

Stop leaking cached envoy snapshots.

For #7086

API changes

Code changes

CI changes

Docs changes

Context

Interesting decisions

Testing steps

Notes for reviewers

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

BOT NOTES:
resolves https://github.com/solo-io/solo-projects/issues/7086

For #7086

Signed-off-by: David L. Chandler <david.chandler@solo.io>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 6e54c42):

https://gloo-edge--pr11309-chandler-clearsnapsh-7llijtwk.web.app

(expires Tue, 11 Aug 2026 22:24:01 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

…atus GC

Rework the snapshot eviction and stale-route-status GC on this branch
based on review findings.

xds eviction:
- Register eviction on uniqueClients delete events (the refcounted
  "last stream gone" signal) instead of perclientSnapCollection deletes,
  which also fire transiently for still-connected clients when
  snapshotPerClient returns nil (translation blips, the
  defer-building-snapshot hack). Clearing on those would withdraw a live
  envoy's last coherent config and force-close its watches, resetting
  healthy ADS streams with codes.Unavailable.
- Guard ClearSnapshot with GetStatusInfo instead of GetSnapshot: the
  status entry is what ClearSnapshot nil-derefs when absent (control
  plane panic), and the GetSnapshot probe deep-cloned the entire
  snapshot just to test existence.

route status GC:
- Skip (and keep tracking) routes that still reference a live Gateway
  of a managed class: absence from the report is then likely a
  transient translation failure, and stripping status would wipe valid
  Accepted/ResolvedRefs conditions off attached, serving routes.
- Read through mgr.GetAPIReader() so conflict retries actually see a
  fresh resourceVersion and a lagging informer cache can never hide our
  own just-written status (which would drop the route from tracking
  with its stale status left behind).
- Collapse the three per-kind tracking fields and mutex into one
  kind-keyed map driven by a table. The GC only runs on the single
  status-sync goroutine, and a mutex alone would not make concurrent
  use safe anyway - correctness also depends on report ordering.
- Simplify removeControllerRouteStatus with slices.DeleteFunc.

Add a regression test covering the still-referenced-gateway guard.

Signed-off-by: David L. Chandler <david.chandler@solo.io>
@solo-changelog-bot

Copy link
Copy Markdown

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/7086

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant