Skip to content

lua: splited to listener and route filter for memory spikes#9368

Open
zirain wants to merge 7 commits into
envoyproxy:mainfrom
zirain:per-route-lua-without-filterchain
Open

lua: splited to listener and route filter for memory spikes#9368
zirain wants to merge 7 commits into
envoyproxy:mainfrom
zirain:per-route-lua-without-filterchain

Conversation

@zirain

@zirain zirain commented Jun 29, 2026

Copy link
Copy Markdown
Member

Fixes: #9355

  • Added a Luas field to the listener-level EnvoyExtensions struct so listener-scoped Lua policies can be represented separately from per-route ones.
  • Refactored to add HCM placeholder filters for both per-route Lua slots and listener-level Lua filters (using named Lua HCM filters with empty default source). Listener filters are appended after route-slot filters to preserve ordering after HCM filter reversal. The Disabled: true flag was removed from listener Lua HCM filters since they are always active for the listener.
  • New implementation that injects listener-level Lua source code and filter context into RouteConfigurationVirtualHost.TypedPerFilterConfig, delivering script changes via RDS to avoid listener drains.

With this patch, for envoy that has a listener with ~1000 routes, the memory cost reduce from 1GB to 120MB.

TODO: add stats check after envoyproxy/envoy#45871 merged.

@zirain zirain requested a review from a team as a code owner June 29, 2026 05:19
@netlify

netlify Bot commented Jun 29, 2026

Copy link
Copy Markdown

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 4854bf6
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a47b8eeb37e4a00081df7cf
😎 Deploy Preview https://deploy-preview-9368--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a330657c66

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal/xds/translator/lua.go Outdated
Comment thread internal/xds/translator/lua.go Outdated
Comment thread internal/xds/translator/translator.go Outdated
@zirain zirain force-pushed the per-route-lua-without-filterchain branch 2 times, most recently from c4a1e4a to ad3785a Compare June 29, 2026 05:28
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.96241% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.38%. Comparing base (dc73f48) to head (4854bf6).

Files with missing lines Patch % Lines
internal/xds/translator/lua.go 80.32% 7 Missing and 5 partials ⚠️
internal/gatewayapi/envoyextensionpolicy.go 76.47% 2 Missing and 2 partials ⚠️
internal/xds/translator/httpfilters.go 77.77% 1 Missing and 1 partial ⚠️
internal/xds/translator/translator.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9368      +/-   ##
==========================================
+ Coverage   75.33%   75.38%   +0.04%     
==========================================
  Files         252      252              
  Lines       41471    41570      +99     
==========================================
+ Hits        31243    31337      +94     
+ Misses       8115     8113       -2     
- Partials     2113     2120       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zirain zirain force-pushed the per-route-lua-without-filterchain branch from 9e1e5ca to 9b269cb Compare June 29, 2026 06:40
@zhaohuabing

zhaohuabing commented Jun 29, 2026

Copy link
Copy Markdown
Member

Is this finding legitimate?

[High] Listener-level Lua leaks onto routes claimed by a more-specific non-Lua policy — internal/xds/translator/lua.go:169-171

if irRoute.EnvoyExtensions == nil || len(irRoute.EnvoyExtensions.Luas) == 0 {
return nil // <-- bails before disabling the inherited listener Lua
}

Trigger:

  1. A route-level EnvoyExtensionPolicy sets only ExtProc/Wasm on route R → translateEnvoyExtensionPolicyForRoute sets r.EnvoyExtensions = {ExtProcs:…, Luas:nil} (envoyextensionpolicy.go:607).
  2. A gateway-level Lua policy runs → gateway loop puts the script at listener scope (http.EnvoyExtensions.Luas, line 699) and skips R because r.EnvoyExtensions != nil (line 708). Per the code's own precedence rule ("a more-specific scope wins," line 704), R must be fully excluded from the gateway policy.
  3. But the listener Lua is delivered via patchVirtualHost → vHost TypedPerFilterConfig, which Envoy inherits into every route, including R.
  4. patchRoute(R) sees len(Luas)==0 → early-returns → never emits lua/listener/0: disabled:true for R.

Result: R runs the gateway Lua it should have overridden. Pre-PR this couldn't happen (gateway Lua was applied per-route and the same line-708 guard kept it off R). So it's a genuine regression.

What makes it more than a one-liner: routes that legitimately should inherit the gateway Lua (no route policy → gateway loop sets r.EnvoyExtensions = {ExtProcs,Wasms,DynamicModules}, Luas nil at line 717) are indistinguishable in the IR from R at patchRoute time — both have EnvoyExtensions != nil and Luas == nil. The listener-Lua design dropped the "this route was excluded by precedence" signal, so the fix needs to re-introduce that distinction (e.g. mark precedence-excluded routes in the IR), not just tweak the if.

@zirain

zirain commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

Is this finding legitimate?

[High] Listener-level Lua leaks onto routes claimed by a more-specific non-Lua policy — internal/xds/translator/lua.go:169-171

if irRoute.EnvoyExtensions == nil || len(irRoute.EnvoyExtensions.Luas) == 0 { return nil // <-- bails before disabling the inherited listener Lua }

Trigger:

  1. A route-level EnvoyExtensionPolicy sets only ExtProc/Wasm on route R → translateEnvoyExtensionPolicyForRoute sets r.EnvoyExtensions = {ExtProcs:…, Luas:nil} (envoyextensionpolicy.go:607).
  2. A gateway-level Lua policy runs → gateway loop puts the script at listener scope (http.EnvoyExtensions.Luas, line 699) and skips R because r.EnvoyExtensions != nil (line 708). Per the code's own precedence rule ("a more-specific scope wins," line 704), R must be fully excluded from the gateway policy.
  3. But the listener Lua is delivered via patchVirtualHost → vHost TypedPerFilterConfig, which Envoy inherits into every route, including R.
  4. patchRoute(R) sees len(Luas)==0 → early-returns → never emits lua/listener/0: disabled:true for R.

Result: R runs the gateway Lua it should have overridden. Pre-PR this couldn't happen (gateway Lua was applied per-route and the same line-708 guard kept it off R). So it's a genuine regression.

What makes it more than a one-liner: routes that legitimately should inherit the gateway Lua (no route policy → gateway loop sets r.EnvoyExtensions = {ExtProcs,Wasms,DynamicModules}, Luas nil at line 717) are indistinguishable in the IR from R at patchRoute time — both have EnvoyExtensions != nil and Luas == nil. The listener-Lua design dropped the "this route was excluded by precedence" signal, so the fix needs to re-introduce that distinction (e.g. mark precedence-excluded routes in the IR), not just tweak the if.

good catch.

@jukie

jukie commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Once this is merged please ping me or cherry-pick yourself to 1.8 branch

@zirain

zirain commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2513fdaa16

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal/xds/translator/lua.go Outdated
@zirain zirain force-pushed the per-route-lua-without-filterchain branch from 2513fda to 843a8c1 Compare June 30, 2026 08:15
@zirain

zirain commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 843a8c153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal/xds/translator/lua.go Outdated
Comment thread internal/gatewayapi/envoyextensionpolicy.go
@zirain

zirain commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8dc11cf79

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal/xds/translator/lua.go
Comment thread internal/xds/translator/lua.go
@zirain

zirain commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe6ae4ee69

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

ExtProcs: extProcs,
Wasms: wasms,
DynamicModules: dynamicModules,
FromGatewayPolicy: new(true),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replace invalid bool allocation

This gateway-policy path now uses new(true), but Go's new operand must be a type rather than a value, so any build of internal/gatewayapi after this commit fails before tests can run. Use an existing bool-pointer helper such as ptr.To(true) or assign true to a local bool and take its address.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this's valid in go 1.26

zirain added 3 commits July 3, 2026 21:28
- Added a Luas field to the listener-level EnvoyExtensions struct so listener-scoped Lua policies can be represented separately from per-route ones.
- Refactored to add HCM placeholder filters for both per-route Lua slots and listener-level Lua filters (using named Lua HCM filters with empty default source). Listener filters are appended after route-slot filters to preserve ordering after HCM filter reversal. The Disabled: true flag was removed from listener Lua HCM filters since they are always active for the listener.
- New implementation that injects listener-level Lua source code and filter context into RouteConfiguration.TypedPerFilterConfig, delivering script changes via RDS to avoid listener drains.

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
zirain added 4 commits July 3, 2026 21:28
…on-Lua policy

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain force-pushed the per-route-lua-without-filterchain branch from fe6ae4e to 4854bf6 Compare July 3, 2026 13:28
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.

Gateway-targeted EnvoyExtensionPolicy Lua is materialized per route (LuaPerRoute) since v1.7.2: proxy memory scales with route count x worker threads

3 participants