marchuang-neoland commented on issue #12275: URL: https://github.com/apache/apisix/issues/12275#issuecomment-6677487529
Locally verified the before/after behavior using the real OpenResty shared dictionary. ### Source revisions Both runs use unmodified upstream `prometheus_keys.lua`: - Before: [d7d8df4627ea5af2749e6ed03426a2238bb24cc7](https://github.com/api7/nginx-lua-prometheus/blob/d7d8df4627ea5af2749e6ed03426a2238bb24cc7/prometheus_keys.lua), the parent of the fix. - After: [f8d6e9fefd4e6d104439047b8638630ade5e13ab](https://github.com/api7/nginx-lua-prometheus/commit/f8d6e9fefd4e6d104439047b8638630ade5e13ab), merged via api7/nginx-lua-prometheus#16. - APISIX integration: #13708 upgrades the dependency to `0.20260716`. ### Setup and fault injection OpenResty `1.27.1.2-0-alpine-fat`, a real `prometheus_metrics 10m` shared dict, one worker, identical harness for both revisions. The fault injection follows the reproduction already posted above: ```lua local KeyIndex = require("prometheus_keys") local dict = ngx.shared.prometheus_metrics local ki = KeyIndex.new(dict, "__ngx_prom__") for i = 1, 50 do ki:add("series_" .. i, "dict full") end -- Simulate counter eviction while index slots survive. dict:delete("__ngx_prom__key_count") ki:add("new_series", "dict full") ``` A forwarding wrapper delegates operations to the real shared dict and aborts before the 200,001st add operation, preventing the old code from hanging indefinitely. ### Results | Scenario | Before fix | After fix | | --- | --- | --- | | Healthy registration | Pass | Pass | | Counter missing, 50 surviving slots | 200,000 collisions on the same slot; watchdog abort | Returns after 52 add attempts; ~0.16 ms CPU time | | Fresh KeyIndex instance after counter loss, simulating worker restart | Same spin | Recovers; ~0.17 ms CPU time | | Counter missing, 1,500 surviving slots | Same spin | Returns a degradation error after 1,000 repairs; the next call completes recovery | All eight cases matched expectations. Successful recovery was checked for preservation of every original series, presence of the new series, and absence of duplicate keys. This confirms the behavioral difference in `KeyIndex:add()`: the old implementation retries an occupied slot without advancing the missing counter; the fixed implementation advances it on repeated collisions and bounds repair work per call. Scope: this is a single-worker, deterministic counter-loss test using a real shared dict. It does not measure natural LRU-eviction frequency, multi-worker lock contention, or end-to-end APISIX behavior. The timings are illustrative local measurements, not a performance benchmark. -- 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]
