AlinsRan commented on code in PR #14061:
URL: https://github.com/apache/apisix/pull/14061#discussion_r4346883710
##########
apisix/plugins/oas-validator.lua:
##########
@@ -176,6 +184,45 @@ local function fetch_and_compile(conf)
return validator
end
+local function get_fresh_shared_entry(cache_key, ttl)
+ local entry = shared_spec_url_entries[cache_key]
+ if entry and ngx_now() - entry.fetched_at < ttl then
+ return entry
+ end
+end
+
+local function create_spec_url_entry(conf, cache_key, ttl)
+ local entry = get_fresh_shared_entry(cache_key, ttl)
+ if entry then
+ return entry
+ end
+
+ -- spec_url_lrucache only serialises per resource, so resources sharing a
+ -- cold URL would otherwise each fetch and keep their own compiled copy
+ local lock, err = resty_lock:new("lrucache-lock")
+ if not lock then
+ return nil, "failed to create lock: " .. err
+ end
+
+ local elapsed, lock_err = lock:lock(plugin_name .. "#" .. cache_key)
+ if not elapsed then
+ return nil, "failed to acquire the lock: " .. lock_err
+ end
+
+ entry = get_fresh_shared_entry(cache_key, ttl)
+ if not entry then
+ local validator
+ validator, err = fetch_and_compile(conf)
+ if validator then
+ entry = {validator = validator, fetched_at = ngx_now()}
+ shared_spec_url_entries[cache_key] = entry
Review Comment:
Refreshing a shared document creates a new `entry`, but only updates the
current resource's cache slot. Other resources still hold the old entry, so the
weak-valued table cannot release its compiled validator. If those resources
stop receiving requests, the old objects remain until cache eviction.
In a reproduction with one route refreshing continuously and three other
routes holding successive versions, three refreshes followed by full GC leave
**four compiled validators** alive. The base version retains only one.
This means that even resources using a single URL can accumulate multiple
compiled copies, substantially increasing worker memory usage for large
specifications.
##########
apisix/plugins/oas-validator.lua:
##########
@@ -234,11 +281,16 @@ local function get_validator(conf)
end
cache_key = cache_key .. "#" .. ngx_md5(tab_concat(parts, "&"))
end
- local validator, err = lrucache(cache_key, nil, fetch_and_compile, conf)
- if not validator then
+
+ -- Key by the declaring resource with the URL key as version, so a changed
+ -- spec_url replaces the resource's slot instead of adding another one.
+ local entry, err = lrucache(conf._meta.parent.resource_key, cache_key,
+ create_spec_url_entry, conf, cache_key,
+ spec_url_lrucache_ttl)
Review Comment:
When a resource first reuses a shared document, its cache slot receives a
full TTL rather than the document's remaining lifetime. Subsequent hits on that
slot do not check `fetched_at`, so an expired document can continue serving
requests without triggering a background refresh.
For example, with a one-second TTL:
- At t=0, A loads the document, which should expire at t=1.
- At t=0.65, B reuses it, but B's cache slot does not expire until t=1.65.
- At t=1.2, a request to B still hits the cache and does not schedule a
refresh.
In the reproduction, running the refresh callback at this point leaves the
fetch count at one, while the base version has already refetched the document.
Validation-rule updates can therefore be delayed by nearly another full
TTL—almost an hour with the default setting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
