AlinsRan opened a new pull request, #13981:
URL: https://github.com/apache/apisix/pull/13981

   ### Description
   
   Follow-up to #13754 / #13658.
   
   Expired entries in the `prometheus-metrics` shared dict are only logically 
dead: every dict API reports them as missing, but their slab pages stay 
allocated. The passive per-write expiry scan cannot reclaim them, because it 
stops at the first non-expired entry at the LRU tail and a permanent entry (the 
error metric, or any metric registered without an `expire`) inevitably ends up 
sitting there. The dict then fills up with dead entries and starts 
force-evicting live ones, which is #13658.
   
   `nginx-lua-prometheus-api7` 1.0.0 fixed that by calling `flush_expired()` 
from `remove_expired_keys()`. Two properties of that call are worth addressing, 
as raised in 
https://github.com/apache/apisix/issues/13658#issuecomment-5139081617:
   
   - it is unbounded, and it holds the shared dict lock for the whole LRU walk, 
so an hour's backlog is reclaimed in a single uninterrupted hold;
   - it runs in every worker, although one process reclaiming is enough.
   
   This PR drains the dict from the privileged agent, in bounded batches:
   
   - `flush_expired(10000)` per call, with a 1s pause between batches, at most 
30 batches per tick;
   - a call that frees less than a batch has already walked the whole queue, so 
the loop stops there;
   - the timer only starts when at least one metric is configured with an 
`expire`, since nothing in the dict expires otherwise;
   - the interval is configurable through 
`plugin_attr.prometheus.flush_expired_interval`, default 60s.
   
   The library timer still runs hourly in each worker, but with the backlog 
already drained it finds nothing left to reclaim, so its walk is cheap.
   
   Measured on OpenResty 1.29.2.4 with a 512m dict, a permanent entry pinning 
the LRU tail (single process, `resty`, no lock contention):
   
   | entries | expired | lock hold of one `flush_expired()` |
   |---|---|---|
   | 301k | 1k | ~3ms (walks the whole queue) |
   | 750k | 749.7k | ~70ms, unbounded |
   | 750k | 749.7k | ~1ms per `flush_expired(10000)` call |
   
   Reclaim work is ~0.09µs per entry freed and ~0.01µs per live node walked, so 
a batch of 10000 is ~1ms.
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [x] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, 
please discuss on the [APISIX mailing 
list](https://github.com/apache/apisix/tree/master#community) first)
   


-- 
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]

Reply via email to