Conversation
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, so the dict fills up with dead entries and starts force-evicting live ones (apache#13658). nginx-lua-prometheus-api7 1.0.0 reclaims them from every worker on an hour-long timer with an unbounded `flush_expired()` call. That call holds the dict lock for the whole LRU walk, so a backlog that built up over an hour is reclaimed in one uninterrupted hold, and every worker repeats the walk. Drain them from the privileged agent instead, in batches of 10000 with a pause in between, so no single call can hold the lock for long: a batch is ~1ms of reclaim work, and a call that finds less than a batch has already walked the whole queue. The library timer still runs, but by then there is nothing left for it to reclaim. The timer only starts when at least one metric is configured with an `expire`, since nothing in the dict expires otherwise. Its interval is configurable through `plugin_attr.prometheus.flush_expired_interval`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #13754 / #13658.
Expired entries in the
prometheus-metricsshared 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 anexpire) 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-api71.0.0 fixed that by callingflush_expired()fromremove_expired_keys(). Two properties of that call are worth addressing, as raised in #13658 (comment):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;expire, since nothing in the dict expires otherwise;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):flush_expired()flush_expired(10000)callReclaim work is ~0.09µs per entry freed and ~0.01µs per live node walked, so a batch of 10000 is ~1ms.
Checklist