From 8e41920bf0b2354f428645876e452a53a7db8316 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Wed, 23 Sep 2026 04:46:41 +0800 Subject: [PATCH] fix(prometheus): reclaim expired metric entries in bounded batches 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 (#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`. --- apisix/plugins/prometheus/exporter.lua | 120 +++++++++++++++++++++++++ conf/config.yaml.example | 3 + t/plugin/prometheus-metric-expire.t | 31 +++++++ 3 files changed, 154 insertions(+) diff --git a/apisix/plugins/prometheus/exporter.lua b/apisix/plugins/prometheus/exporter.lua index fbb600e18687..0fdfbb3049ca 100644 --- a/apisix/plugins/prometheus/exporter.lua +++ b/apisix/plugins/prometheus/exporter.lua @@ -89,6 +89,21 @@ end -- Default refresh interval local DEFAULT_REFRESH_INTERVAL = 15 +-- Default interval for reclaiming expired entries from the metrics shared dict +local DEFAULT_FLUSH_EXPIRED_INTERVAL = 60 + +-- Entries a single flush_expired() call may reclaim. The call holds the shared +-- dict lock until it returns, so this is what bounds how long the workers can +-- be kept waiting on that lock: measured at ~1ms per 10000 entries. +local FLUSH_EXPIRED_BATCH = 10000 + +-- Upper bound on the batches run by one tick, so the loop always finishes well +-- within one interval. Whatever is left over is picked up by the next tick. +local FLUSH_EXPIRED_MAX_BATCHES = 30 + +-- Pause between batches, so the lock is not taken back to back +local FLUSH_EXPIRED_BATCH_DELAY = 1 + local CACHED_METRICS_KEY = "cached_metrics_text" local metrics = {} @@ -99,6 +114,10 @@ local exporter_timer_running = false local exporter_timer_created = false +local flush_expired_timer_running = false + +local flush_expired_timer_created = false + local function gen_arr(...) clear_tab(inner_tab_arr) for i = 1, select('#', ...) do @@ -1201,6 +1220,105 @@ local function exporter_timer(premature, yieldable, cache_exptime) end +-- Expired entries in the metrics shared dict are only logically dead: every +-- dict API reports them as missing, but their slab pages stay allocated until +-- something reclaims them. The passive per-write expiry scan cannot do it, +-- 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. Left alone, the dict fills up with +-- dead entries and starts evicting live ones (apache/apisix#13658). The metrics +-- library reclaims them from every worker once an hour with an unbounded +-- flush_expired() call, which walks the whole LRU queue with the dict lock +-- held. Draining them here instead -- in the privileged agent, in bounded +-- batches -- keeps that backlog, and therefore the lock hold of any single +-- call, small. +local function flush_expired_metrics() + local dict = ngx.shared["prometheus-metrics"] + if not dict then + return 0 + end + + local total = 0 + for _ = 1, FLUSH_EXPIRED_MAX_BATCHES do + -- a return value below the batch size means this call has already + -- walked the whole LRU queue, so there is nothing left to reclaim + local freed = dict:flush_expired(FLUSH_EXPIRED_BATCH) + total = total + freed + if freed < FLUSH_EXPIRED_BATCH then + break + end + + ngx.sleep(FLUSH_EXPIRED_BATCH_DELAY) + end + + return total +end +_M.flush_expired_metrics = flush_expired_metrics + + +local function flush_expired_timer(premature) + if premature then + return + end + + if flush_expired_timer_running then + core.log.warn("Previous metrics flush still running, skipping") + return + end + + flush_expired_timer_running = true + + local ok, err = pcall(flush_expired_metrics) + if not ok then + core.log.error("Failed to flush expired metrics: ", err) + end + + flush_expired_timer_running = false +end + + +-- Nothing in the dict ever becomes expired unless at least one metric is +-- registered with an expire, so the timer is only worth running then. +local function metrics_expire_enabled(attr) + local metrics_attr = attr and attr.metrics + if type(metrics_attr) ~= "table" then + return false + end + + for _, conf in pairs(metrics_attr) do + local expire = type(conf) == "table" and tonumber(conf.expire) + if expire and expire > 0 then + return true + end + end + + return false +end + + +local function init_flush_expired_timer(attr) + if flush_expired_timer_created or not metrics_expire_enabled(attr) then + return + end + + local interval = DEFAULT_FLUSH_EXPIRED_INTERVAL + if attr and attr.flush_expired_interval then + interval = attr.flush_expired_interval + end + + if interval <= 0 then + return + end + + local ok, err = ngx_timer_every(interval, flush_expired_timer) + if ok then + flush_expired_timer_created = true + else + core.log.error("Failed to start the metrics flush timer: ", err) + end +end + + local function init_exporter_timer() if process.type() ~= "privileged agent" then return @@ -1214,6 +1332,8 @@ local function init_exporter_timer() local cache_exptime = refresh_interval * 2 + init_flush_expired_timer(attr) + exporter_timer(false, false, cache_exptime) if exporter_timer_created then diff --git a/conf/config.yaml.example b/conf/config.yaml.example index 6a1361aec496..bb93121fdc38 100644 --- a/conf/config.yaml.example +++ b/conf/config.yaml.example @@ -724,6 +724,9 @@ plugin_attr: # Plugin attributes ip: 127.0.0.1 # Set the IP. port: 9091 # Set the port. refresh_interval: 15 # Set the interval for refreshing cached metric data. unit: second. + flush_expired_interval: 60 # Set the interval for reclaiming expired entries from the + # `prometheus-metrics` shared dict. Only used when a metric + # below is configured with `expire`. unit: second. # metrics: # Create extra labels from nginx variables: https://nginx.org/en/docs/varindex.html # http_status: # expire: 0 # The expiration time after which metrics are removed. unit: second. diff --git a/t/plugin/prometheus-metric-expire.t b/t/plugin/prometheus-metric-expire.t index caad85ea04a5..dd30201ff390 100644 --- a/t/plugin/prometheus-metric-expire.t +++ b/t/plugin/prometheus-metric-expire.t @@ -130,3 +130,34 @@ plugin_attr: GET /t --- response_body passed + + + +=== TEST 2: expired entries are reclaimed in bounded batches +--- config + location /t { + content_by_lua_block { + local exporter = require("apisix.plugins.prometheus.exporter") + local dict = ngx.shared["prometheus-metrics"] + + -- a permanent entry kept at the LRU tail stops the passive + -- per-write expiry scan, which is what lets expired entries pile + -- up in the first place + dict:set("flush_expired_tail", 1) + for i = 1, 20000 do + dict:set("flush_expired_" .. i, "v", 0.1) + end + ngx.sleep(0.2) + + -- more than one batch, so the loop has to run again after the + -- delay instead of stopping at the first batch + local freed = exporter.flush_expired_metrics() + ngx.say("reclaimed all: ", freed >= 20000) + ngx.say("left over: ", exporter.flush_expired_metrics()) + ngx.say("tail kept: ", dict:get("flush_expired_tail")) + } + } +--- response_body +reclaimed all: true +left over: 0 +tail kept: 1