Dedup DNS attribution rows in one pass, not per candidate row - #741
Merged
Conversation
getQAName() deduplicated a shared IP's DNS evidence with a correlated scalar subquery: for every candidate row of the IP, SQLite re-searched idx_dns_resource over all of that IP's rows and sorted them in a temp B-tree to find the freshest row per qname. That is O(n^2) row visits plus one sort per row, it grows with accumulated DNS history (rows live for at least the TTL floor, three days by default), and the query runs for every new connection: once from log() with the alive filter off, and once from blockKnownTracker() on an ipToHost cache miss, inside the is_address_allowed upcall from the packet path. On IPs shared by hundreds of qnames this turns every connection into hundreds of thousands of row visits, which surfaces as sustained CPU, heat, and battery drain that worsens as the dns table fills. Replace the correlated subquery with a single MAX(time) aggregate grouped by qname. With exactly one min/max aggregate, SQLite takes the bare columns from the row that supplied the maximum, so the dedup semantics from 6c34ce1 are unchanged - freshest row per qname, qnames ordered by recency - while the plan collapses to one index range scan over the IP's rows. In a 1,200-row shared-IP benchmark this drops the lookup from ~147 ms to ~0.9 ms. Also cover the alive-filter interplay: an expired fresher row must not shadow an older row that is still alive. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Hp2Xpr5CSahi9Qizg9zjp
kasnder
marked this pull request as ready for review
August 20, 2026 20:30
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
2 participants
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
Root-causes the battery/heat regression attributed to "DNS deduping". The culprit is not #739 (the DNS response-rewriting dedup — reviewed below), but the qname dedup that #690 added to
DatabaseHelper.getQAName().#690 deduplicates a shared IP's DNS evidence with a correlated scalar subquery: for every candidate row of the IP, SQLite re-searches
idx_dns_resourceacross all of that IP's rows and sorts them in a temp B-tree to pick the freshest row per qname (EXPLAIN QUERY PLAN:CORRELATED SCALAR SUBQUERY→SEARCH d2 USING INDEX idx_dns_resource (resource=?)→USE TEMP B-TREE FOR ORDER BY, executed once per outer row). That is O(n²) row visits plus one sort per row, where n is the number of dns rows sharing the IP.Two things make this a battery/heat issue rather than a slow query:
ServiceSinkhole.log()callsgetQAName(uid, daddr, false)for every logged packet event — with the alive filter off, so n includes the full accumulated history for that IP.blockKnownTracker()runs it again (alive=true) on everyipToHostcache miss, inside theis_address_allowedJNI upcall from the native packet path.insertDns()clamps TTL to the "ttl" preference floor (3 days by default) and expired rows are only removed by cleanup, so hot CDN/tracker IPs steadily accumulate qname rows. The regression creeps in as the table fills — matching a heat report that surfaces weeks after Attribute shared-IP DNS evidence by recency, not alphabetically #690 merged.Benchmark (real SQLite, 400 qnames × 3 anames on one shared IP = 1,200 rows): ~147 ms per lookup before, ~0.9 ms after (~150×) — per connection, on the log handler thread and the packet-path upcall.
Fix
Replace the correlated subquery with a single
MAX(time)aggregate grouped by qname. With exactly one min/max aggregate, SQLite takes the bare columns from the row that supplied the maximum, so #690's semantics are unchanged — freshest row per qname, qnames ordered by recency — while the plan collapses to one index range scan over the IP's rows (same shape as before #690, now with recency ordering).Validation
EXPLAIN QUERY PLANconfirms the correlated subquery is gone.insertDns()'s TTL clamp would keep the fresh row alive).:app:testGithubDebugUnitTestruns in CI on this PR; I'll watch it.Review of #739 for the same symptom
"Deduplicate DNS response rewriting" (#739) was also examined for battery/heat mechanisms and none was found: the
tc-dnsparse is bounded per packet (name-compression depth 8, 255-octet cap), it only runs for port-53 traffic, the Rust library is still built--release, no periodic work, timers, or wakeup sources are added, and the C TCP path's new length-prefix rewrite only shortens isolated complete frames with consistent sequence accounting. Its per-response cost (heap allocation + the same one-per-response JNI upcalls) is bounded by DNS traffic and cannot produce sustained CPU.🤖 Generated with Claude Code
https://claude.ai/code/session_012Hp2Xpr5CSahi9Qizg9zjp
Generated by Claude Code