From 6984425c9b0462429cb8bdd637094b337ca0834f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 20:17:38 +0000 Subject: [PATCH] Dedup DNS attribution rows in one pass, not per candidate row 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 6c34ce18 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 Claude-Session: https://claude.ai/code/session_012Hp2Xpr5CSahi9Qizg9zjp --- .../eu/faircode/netguard/DatabaseHelper.java | 23 +++++++++------- .../DatabaseHelperDnsAttributionTest.java | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/eu/faircode/netguard/DatabaseHelper.java b/app/src/main/java/eu/faircode/netguard/DatabaseHelper.java index 5623fad8..e4eb8fea 100644 --- a/app/src/main/java/eu/faircode/netguard/DatabaseHelper.java +++ b/app/src/main/java/eu/faircode/netguard/DatabaseHelper.java @@ -1166,7 +1166,7 @@ public Cursor getQAName(int uid, String ip, boolean alive) { SQLiteDatabase db = readableDb; String escapedIp = ip.replace("'", "''"); String aliveFilter = alive - ? " AND (%1$s.time IS NULL OR %1$s.time + %1$s.ttl >= " + now + ")" + ? " AND (d.time IS NULL OR d.time + d.ttl >= " + now + ")" : ""; // There is a segmented index on resource. A shared IP can carry // DNS evidence for several qnames; keep only the most recently @@ -1174,17 +1174,20 @@ public Cursor getQAName(int uid, String ip, boolean alive) { // the freshest resolution — most likely tied to the connection // that's actually being made now — is attributed first instead // of an alphabetically-first but possibly stale one. - String query = "SELECT d.qname, d.aname, d.time, d.ttl" + + // + // The dedup deliberately uses a single MAX(time) aggregate: with + // exactly one min/max aggregate, SQLite takes the bare columns + // from the row that supplied the maximum, so this is one index + // range scan over the IP's rows. A correlated per-row subquery + // here re-scans the IP's rows once per candidate row — O(n²) — + // and this query runs for every new connection (log() and + // blockKnownTracker()), where it grows with DNS history until + // it shows up as battery drain and heat. + String query = "SELECT d.qname, d.aname, d.time, d.ttl, MAX(d.time)" + " FROM dns AS d" + " WHERE d.resource = '" + escapedIp + "'" + - String.format(aliveFilter, "d") + - " AND d.ID = (" + - " SELECT d2.ID FROM dns AS d2" + - " WHERE d2.resource = d.resource AND d2.qname = d.qname" + - String.format(aliveFilter, "d2") + - " ORDER BY d2.time DESC, d2.ID DESC" + - " LIMIT 1" + - " )" + + aliveFilter + + " GROUP BY d.qname" + " ORDER BY d.time DESC, d.ID DESC"; return db.rawQuery(query, new String[] {}); } finally { diff --git a/app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java b/app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java index e7878f97..2cfbbf13 100644 --- a/app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java +++ b/app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java @@ -59,6 +59,33 @@ public void repeatedObservationsOfSameQnameCollapseToFreshestRow() { } } + @Test + public void aliveFilterAppliesBeforeDedup() { + DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication()); + dh.clearDns(); + + String ip = "203.0.113.30"; + // The freshest observation has already expired; an older one is still + // alive. With alive=true the expired row must not shadow the alive one. + // Rows are inserted directly because insertDns() clamps the TTL to the + // "ttl" preference floor, which would keep the fresh row alive. + long now = System.currentTimeMillis(); + dh.getWritableDatabase().execSQL( + "INSERT INTO dns (time, qname, aname, resource, ttl) VALUES (" + + (now - 10_000) + ", 't2.example.com', 'alive-cname.example.com', '" + + ip + "', 3600000)"); + dh.getWritableDatabase().execSQL( + "INSERT INTO dns (time, qname, aname, resource, ttl) VALUES (" + + (now - 5_000) + ", 't2.example.com', 'expired-cname.example.com', '" + + ip + "', 1000)"); + + try (Cursor c = dh.getQAName(-1, ip, true)) { + assertEquals(1, c.getCount()); + assertTrue(c.moveToFirst()); + assertEquals("alive-cname.example.com", c.getString(c.getColumnIndexOrThrow("aname"))); + } + } + private static ResourceRecord rr(long time, String qname, String aname, String resource, int ttl) { ResourceRecord rr = new ResourceRecord(); rr.Time = time;