Fix expired hourly query retention#2060
Open
zs311521 wants to merge 1 commit into
Open
Conversation
Open
Member
|
Thanks for the PR. Will evaluate this in detail soon. |
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.
Summary
_hourStat._querieswhen minute detail ages out of the last-hour windowRelated to #2030. This fixes the isolated defect described below; it does not claim that every source of RSS growth in #2030 is resolved.
Root cause
Each locked minute counter is merged into
HourlyStats._hourStat, including its high-cardinality_queriesmap. After an hour leaves the last-hour window,UnloadMinuteStats()releases the 60 minute counters but leaves that merged query map in the hourly cache for up to 24 hours.Cache prefetch builds its last-hour view from
_lastHourStatCountersCopy. The dashboard paths retain their scalar totals and separate domain/client/protocol maps. I found no live reader that needs expired_hourStat._queriesafter minute detail is unloaded; maintainer confirmation of that boundary is welcome.The maintenance threshold excludes the current and previous hour. Hourly files are written from the minute counters before this older cache entry is unloaded, so the change does not rewrite or truncate the persisted hourly file.
Why this shape
The patch clears only
_queries, aConcurrentDictionary; all reporting counters and maps remain intact. It deliberately keeps repeatedUnloadMinuteStats()cleanup: if an unusually late maintenance overlap adds a query after one pass, the next pass removes it instead of returning early.The more invasive alternative—never merging queries into
_hourStat—would lower peak duplication further but has a wider behavior surface. Clear-on-unload is the conservative fix.Verification
dns-server-v15.4.0)Production corroboration
On a busy v15.4 forwarder using concurrent DNS-over-QUIC upstreams, RSS rose from 758.4 MiB to 6,777.9 MiB over 15 hours (about 401 MiB/hour by endpoint slope). An equivalently configured mostly-idle standby stayed near baseline, and a low-volume standalone node remained near 740 MiB after about nine days.
As a short mitigation experiment, enabling in-memory stats and restarting held the busy node to 872.4 MiB at 1 hour 49 minutes. Depending on the comparison window, observed growth was roughly 69–100 MiB/hour rather than ~401 MiB/hour. This is strong evidence that the disk-backed stats path is a major contributor, but the window is short, the restart is a confounder, and residual growth may have other managed or native causes. That is why this PR remains scoped to the reproduced defect and does not close #2030.