Skip to content

fix(traffic-label): cache the compiled match expressions outside the plugin config - #13901

Merged
nic-6443 merged 5 commits into
apache:masterfrom
nic-6443:fix/traffic-label-expr-cache
Sep 1, 2026
Merged

fix(traffic-label): cache the compiled match expressions outside the plugin config#13901
nic-6443 merged 5 commits into
apache:masterfrom
nic-6443:fix/traffic-label-expr-cache

Conversation

@nic-6443

@nic-6443 nic-6443 commented Aug 31, 2026

Copy link
Copy Markdown
Member

Description

traffic-label's access() compiles each rule's match expression on the first request and stores the result in conf.rules_arr — inside the plugin configuration itself.

For the ipmatch operator, resty.expr compiles the right-hand side into an ipmatcher (resty/expr/v1.lua), and an ipmatcher's ipv4 / ipv4_mask lookup tables are keyed by integers (the 32-bit address and the prefix length). cjson refuses to serialise such a table:

failed to encode: Cannot serialise table: excessively sparse array force: true

So from the first request that reaches the plugin onwards, the route configuration can no longer be JSON encoded. With the error log at info level, every line that dumps the route comes out empty and each one emits two warnings (log_wrapper evaluates a __tostring argument twice):

[warn]  json.lua:179: failed to encode: Cannot serialise table: excessively sparse array force: true
[warn]  json.lua:179: failed to encode: Cannot serialise table: excessively sparse array force: true
[info]  init.lua:862: http_access_phase(): matched route:

matched route, insert uri route and route conf are all affected, i.e. the debugging output you turned info on for is exactly what is lost.

Keep the compiled expression on the rule itself but behind a metatable, so it stays invisible to serialisation while its lifetime still matches the configuration exactly — compiled once, no TTL, no eviction cap, no recompilation. This is the same technique #11770 introduced for body-transformer and that set_plugins_meta_parent uses for the plugin _meta.parent.

Which issue(s) this PR fixes

N/A

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (none needed: no user-facing behaviour change)
  • I have verified that this change is backward compatible

…plugin config

access() compiled each rule's `match` expression once and stored the result in
`conf.rules_arr`, i.e. inside the plugin configuration itself. For the `ipmatch`
operator resty.expr compiles into an ipmatcher, whose `ipv4` and `ipv4_mask`
lookup tables are keyed by integers; cjson rejects such a table as an
"excessively sparse array". From the first request that reaches the plugin
onwards the route configuration therefore could no longer be JSON encoded: with
the error log at info level every line that dumps the route (`matched route`,
`insert uri route`, `route conf`) came out empty, and each one emitted two
"failed to encode" warnings.

Cache the compiled expressions in an lrucache keyed by `conf.rules`, the way the
round-robin objects are already cached, and leave the configuration untouched.
Copilot AI lite review requested due to automatic review settings August 31, 2026 14:47

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Keep the compiled expression on the rule itself, behind a metatable, rather than
in a module-level lrucache. cjson does not walk metatables, so the expression
stays invisible to serialisation while its lifetime matches the configuration
exactly: compiled once, no TTL, no eviction cap and no recompilation, which is
what the original `conf.rules_arr` cache provided.

This is the same technique already used to keep runtime fields out of
serialisation in body-transformer (apache#11770) and for the plugin `_meta.parent`.
`rules` items have no `additionalProperties: false`, so a rule may carry any
property, `_expr` included. A truthy value skipped the compilation and a `false`
one shadowed the metatable entry, and either way `rule._expr:eval()` then indexed
a non-table and aborted the request with a 500.

Key the cached expression on a module-local table instead: a table key cannot be
expressed in JSON, so no configuration can collide with it. The same hazard
existed before this branch, through the `conf.rules_arr` field.
JSON null is stored as-is and decodes to a userdata, which is truthy, so it
skips the compilation exactly like `true` does and then aborts the request.
Restructure the block as well: three rules of nested JSON overflowed the
inlined-Lua size limit of `content_by_lua_block`.
This reverts the module-local key and its tests. A rule carrying a property
named `_expr` does abort the request, but the same input shape already does so
on master through `conf.rules_arr`, so this branch is no better or worse than
the current behaviour there. Keeping it out of scope.

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@nic-6443
nic-6443 merged commit 51fad8a into apache:master Sep 1, 2026
17 checks passed
@nic-6443
nic-6443 deleted the fix/traffic-label-expr-cache branch September 1, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants