fix(keys): keep a returning metric on its slot, and stop broadcasting

[AlinsRan]

Stacked on #23, which it needs for auto_flush_expired. Design, scenario

A metric that expired and came back consumed a new slot number and bumped
delete_count, which sends every worker through a full sync of key_count
slots — on the request path, since sync() is the first thing add() does and
add() runs on every observation of a metric with an exptime.

Reproduced on the shape of a gateway pod: 10 workers, a 512m dict, the three
metrics of apisix/plugins/prometheus/exporter.lua with their label sets and the
default latency buckets, 140k label combinations, 200 req/s of exactly what
exporter.http_log() does. CPU summed over the workers, 10s windows:

	v1.0.0	this PR
steady 200 req/s	14.9%	15.9%
one low-frequency series comes back	30.2% (delete_count +1)	22.1% (+0)
50 of them over 10s	287.7%, busiest 98.6 / 93.3 / 87.9%	15.7%, busiest 4.0%

The third row is bumped externally, and this PR does not react at all: its
request path no longer reads the shared counters.

What this fixes

	symptom	root cause	change	measured
A. slot numbers grow	key_count climbs monotonically, so a worker's catch-up sync after a reload gets slower and never comes back down	a slot that is only past its ttl is taken for gone, giving up a number that expire() could still have resurrected	judge the slot with ttl(), not get()	100 metrics expiring and coming back: +100 numbers → +0
B. and one broadcast multiplies it	a single delete_count bump makes every worker forget every slot that is merely past its ttl, so all of those metrics take new numbers too	the full sync those workers then run decides with get(), which cannot tell that state from a reclaimed node	remove the broadcast, and the new judgement means even an external bump no longer discards those slots	external delete_count bump: +100 numbers → +0
C. workers peg a core	the top picture in the reports: one worker at ~100% CPU	one low-frequency series returning bumps delete_count, which sends all 10 workers through a full sync of key_count slots — on the request path	remove the broadcast; renewal no longer reads the shared counters at all	one series returning 30.2% → 22.1%; 50 external bumps 287.7% → 15.7% (busiest worker 98.6% → 4.0%)
D. a metric disappears silently	its value is still in the dict, but it is absent from /metrics	cleaning up a dead slot drops self.index[key] without checking that it still points at that slot — by then it points at the live one	one guard in forget_slot()	without the guard, a peer lists 51 → 50 keys; with it, 51 → 51

A and B are key_count growth; C is the CPU spike; D is what the broadcast was
actually protecting, and the reason it could not simply be deleted. The rest of
this description is the evidence behind each row.

Why the broadcast multiplies slot growth

Row B is the part that is easy to miss, so here it is step by step. delete_count
is not just a CPU cost: it is what turns one metric's return into hundreds of new
slot numbers.

1. Worker A observes metric K whose slot's node has been reclaimed, so
   expire() returns "not found". v1.0.0 gives K a fresh number and
   bumps delete_count.

2. Every other worker's next sync() finds self.deleted ~= delete_count, so it
   runs sync_range(0, key_count) — the whole range — instead of
   sync_range(self.last, key_count).

3. Inside that walk, get() returns nil for a slot that is merely past its
   ttl exactly as it does for one whose node is gone. The old code cannot tell
   them apart, so its elseif self.keys[i] branch drops the local reference for
   both:

   elseif self.keys[i] then
     self.index[self.keys[i]] = nil      -- dropped for state (2) as well
     self.keys[i] = nil

4. So that worker now has no slot for every metric that happens to be expired at
   that moment — even though every one of those nodes is still in the dict and
   expire() would have resurrected it in place.

5. Each of those metrics, on its next observation, finds self.index[key] == nil,
   skips the renewal path entirely and allocates a new number at
   key_count + 1.

Two things make it worse. The worker that bumps does not pay: it raises its own
self.deleted first, so the walk — and the forgetting — lands on the other nine.
And it compounds: every new number raises key_count, which makes the next full
sync more expensive and leaves more dead slots behind, each of which is another
future "not found" and another bump.

The difference between the incremental and the full walk is the whole
multiplier. An incremental sync only covers [self.last, key_count], so without
a bump the damage is confined to the top of the range; the bump widens it to
every slot the worker knows.

Measured on v1.0.0's own code — 100 metrics with a 1s exptime, all of them
expiring and then being observed again:

what happened in between	v1.0.0	this PR
nothing (nodes still in the dict)	+2 numbers	+0
another worker bumped delete_count	+100	+0
the nodes were flushed	+100	+100
both	+100	+100

The first row is what should happen: the metrics come back on their own numbers.
The second row is the broadcast turning that into a hundred new ones. The last
two are the residual case this PR does not address — a number whose node is gone
is not the key's any more, and reusing it is what #24 is for.

Three changes, no new shared state

ttl() tells apart the two states get() reports alike. A slot past its ttl
whose node is still in the dict is the key's own — expire() resurrects it with
its value intact — so the reclaim round and sync_range leave it alone and the
key comes back on the same number. Listing it meanwhile costs nothing:
metric_data() skips a key whose value has expired. Only a slot whose node is
gone (ttl() reports "not found") gives its number up.

That one distinction is most of the bloat, as the table above shows.

Renewal reads its own slot, instead of starting with sync():

if self.dict:get(self.key_prefix .. mine) == key then
  self.dict:expire(self.key_prefix .. mine, exptime)   -- done
end

One dict read where sync() is two, and it touches none of the shared counters,
so an external bump cannot make the request path scan.

The broadcast is gone. What it protected was not the duplicate metrics of
apache/apisix#11934 — the index[key] == idx check in list() does that — but a
peer dropping the index entry of a key that had since moved to another slot,
which hid a live metric from that peer's scrape. Measured on 1.0.0, with the
dead slot below the peer's incremental range:

1.0.0 as shipped        delete_count=1    peer lists 51 -> 51, the key is there
1.0.0 minus the bump    delete_count=nil  peer lists 51 -> 50, the key is gone
1.0.0 minus the bump,
  plus the guard below  delete_count=nil  peer lists 51 -> 51, the key is there

The guard is one line: drop an index entry only while it still points at the slot
being given up. And a fresh slot is above every worker's self.last anyway, so
their incremental sync finds it — nothing has to be broadcast.

What this leaves

A metric whose node was already reclaimed still takes a new number, and numbers
are never reused, so key_count keeps growing with genuinely new label sets and
with returns that come more than a reclaim interval late. That costs one thing:
the full catch-up sync a worker does at startup, key_count × ~0.45µs. Slot
reuse (#24) addresses it and stays deferred — it needs a way to tell the scrape
that a slot below its last changed, which is the only part of this work that
would add shared state.

Verification

10 workers plus the privileged agent, checks run inside the scraping process
against a walk of the dict itself, at 300k series with 1,500 new series/s:

	result
live values rendered exactly once	300,020 of 300,020
duplicates / live values missing	0 / 0
counter values, driven exactly	exact
slot-level: listed / live / duplicates / missing	305,159 / 300,021 / 0 / 0
[error] lines	0

listed_expired: 5138 in that run is the deliberate trade: a key whose node is
still there stays listed, and metric_data() skips it — 5k wasted value reads
against 305k slots, in exchange for not consuming a number.

Unit tests: 50, 49 pass. TestPrometheus.testPrintfTable fails on main as
well, under LuaJIT, and is unrelated. The SimpleDict mock now models the three
slot states (get() no longer prunes, ttl() returns a negative number for a
node past its ttl and "not found" once it is freed, expire() resurrects one,
add() replaces one in place), so the tests that assumed the old semantics were
rewritten rather than patched.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 618a7e69-dee2-4aeb-b35f-66068e269c8e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}