From a2102e2becf9d5ca7bbdaf9159656d68b172c6ce Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 15:17:06 +0000 Subject: [PATCH 01/11] feat(okhttp): concurrent caching DNS resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoids paying the getaddrinfo tax on every HTTP call to the same host. Adds a single-flight, LRU+TTL DNS resolver wired into both the media and relay OkHttp clients via a process-wide shared instance, so resolutions cross-cut images, relays, and NIP-05 lookups. - Per-host coalescing: N concurrent lookups for the same host share one upstream call. - Different hosts proceed in parallel — no global lock around the upstream resolver. - Negative cache (10s) prevents hammering on typos / dead hosts. - Positive cache (5m) survives a feed scroll. --- .../amethyst/service/okhttp/AmethystDns.kt | 155 +++++++++++++ .../service/okhttp/OkHttpClientFactory.kt | 1 + .../okhttp/OkHttpClientFactoryForRelays.kt | 1 + .../service/okhttp/AmethystDnsTest.kt | 204 ++++++++++++++++++ 4 files changed, 361 insertions(+) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt new file mode 100644 index 0000000000..a453864a5e --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.okhttp + +import okhttp3.Dns +import java.net.InetAddress +import java.net.UnknownHostException +import java.util.Collections +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ExecutionException +import java.util.concurrent.TimeUnit + +/** + * Concurrent, caching DNS resolver for OkHttp. + * + * The system resolver call ([InetAddress.getAllByName] used by [Dns.SYSTEM]) is a blocking JNI + * hop into `getaddrinfo`. On a busy feed we may issue dozens of HTTP calls to a handful of hosts + * in the same second; the default behaviour pays the resolver tax once per call and serializes + * the OkHttp dispatcher worker that asked for it. + * + * This resolver adds three things on top of the system resolver: + * + * 1. An LRU + TTL cache, so repeated lookups of the same host short-circuit before touching the + * network. Negative results get a short TTL so a typo doesn't keep hammering DNS. + * 2. Single-flight coalescing: when N OkHttp threads ask for the same host concurrently, only + * one of them performs the upstream lookup. The others block on the same future and pick up + * the result. Without this, ten parallel image requests to the same CDN make ten DNS calls. + * 3. No global lock on the slow path: lookups for *different* hosts proceed in parallel because + * the upstream resolver is invoked outside any monitor. + */ +class AmethystDns( + private val delegate: Dns = Dns.SYSTEM, + private val maxEntries: Int = 256, + positiveTtlMs: Long = TimeUnit.MINUTES.toMillis(5), + negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), +) : Dns { + private val positiveTtlNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlMs) + private val negativeTtlNanos = TimeUnit.MILLISECONDS.toNanos(negativeTtlMs) + + private val cache: MutableMap = + Collections.synchronizedMap( + object : LinkedHashMap(64, 0.75f, true) { + override fun removeEldestEntry(eldest: Map.Entry): Boolean = size > maxEntries + }, + ) + private val inflight = ConcurrentHashMap>>() + + override fun lookup(hostname: String): List { + cache[hostname]?.let { entry -> + if (entry.expiresAtNanos > System.nanoTime()) { + return entry.unwrap(hostname) + } + } + + val newFuture = CompletableFuture>() + val existing = inflight.putIfAbsent(hostname, newFuture) + return if (existing == null) { + resolveAsLeader(hostname, newFuture) + } else { + awaitFollower(hostname, existing) + } + } + + private fun resolveAsLeader( + hostname: String, + future: CompletableFuture>, + ): List { + try { + val addresses = delegate.lookup(hostname) + put(hostname, addresses, positiveTtlNanos) + future.complete(addresses) + return addresses + } catch (e: UnknownHostException) { + put(hostname, emptyList(), negativeTtlNanos) + future.completeExceptionally(e) + throw e + } catch (e: Throwable) { + future.completeExceptionally(e) + throw e + } finally { + inflight.remove(hostname, future) + } + } + + private fun awaitFollower( + hostname: String, + future: CompletableFuture>, + ): List { + try { + val addresses = future.get() + return addresses.ifEmpty { throw UnknownHostException(hostname) } + } catch (e: ExecutionException) { + when (val cause = e.cause) { + is UnknownHostException -> throw cause + null -> throw UnknownHostException(hostname) + else -> throw UnknownHostException(hostname).apply { initCause(cause) } + } + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + throw UnknownHostException(hostname).apply { initCause(e) } + } + } + + private fun put( + hostname: String, + addresses: List, + ttlNanos: Long, + ) { + cache[hostname] = Entry(addresses, System.nanoTime() + ttlNanos) + } + + /** Drop all cached entries. Call when the network changes (e.g. WiFi <-> mobile). */ + fun invalidate() { + cache.clear() + } + + /** Drop a single host's cached entry. */ + fun invalidate(hostname: String) { + cache.remove(hostname) + } + + private class Entry( + val addresses: List, + val expiresAtNanos: Long, + ) { + fun unwrap(hostname: String): List = addresses.ifEmpty { throw UnknownHostException(hostname) } + } + + companion object { + /** + * Process-wide instance shared by every OkHttp client built in the app, so a host resolved + * for an image fetch is reused when a relay handshake or NIP-05 lookup hits the same host. + */ + val shared: AmethystDns by lazy { AmethystDns() } + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt index 61f3255f62..6150ad33ff 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt @@ -63,6 +63,7 @@ class OkHttpClientFactory( .Builder() .dispatcher(dispatcher) .connectionPool(connectionPool) + .dns(AmethystDns.shared) .eventListenerFactory(MediaCallEventListenerFactory(dispatcher, connectionPool)) .followRedirects(true) .followSslRedirects(true) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index e4b81b31fb..fb63b34165 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -55,6 +55,7 @@ class OkHttpClientFactoryForRelays( OkHttpClient .Builder() .dispatcher(myDispatcher) + .dns(AmethystDns.shared) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt new file mode 100644 index 0000000000..0c0b88554a --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -0,0 +1,204 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.okhttp + +import okhttp3.Dns +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Test +import java.net.InetAddress +import java.net.UnknownHostException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +class AmethystDnsTest { + private fun ip(value: String) = InetAddress.getByName(value) + + private class CountingDns( + private val responses: Map>, + ) : Dns { + val callsByHost = mutableMapOf() + + override fun lookup(hostname: String): List { + callsByHost.getOrPut(hostname) { AtomicInteger() }.incrementAndGet() + return responses[hostname] ?: throw UnknownHostException(hostname) + } + + fun calls(hostname: String): Int = callsByHost[hostname]?.get() ?: 0 + } + + private class GatedDns( + private val responses: Map>, + ) : Dns { + val started = CountDownLatch(1) + val release = CountDownLatch(1) + val calls = AtomicInteger() + + override fun lookup(hostname: String): List { + calls.incrementAndGet() + started.countDown() + release.await() + return responses[hostname] ?: throw UnknownHostException(hostname) + } + } + + @Test + fun `cache hit avoids second upstream call`() { + val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + val first = dns.lookup("a.example") + val second = dns.lookup("a.example") + + assertEquals(listOf(ip("1.2.3.4")), first) + assertSame(first, second) + assertEquals(1, upstream.calls("a.example")) + } + + @Test + fun `negative cache short-circuits subsequent lookups`() { + val upstream = CountingDns(emptyMap()) + val dns = AmethystDns(delegate = upstream) + + assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } + assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } + + assertEquals(1, upstream.calls("missing.example")) + } + + @Test + fun `positive entries expire`() { + val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 1, + negativeTtlMs = 1, + ) + + dns.lookup("a.example") + Thread.sleep(20) + dns.lookup("a.example") + + assertEquals(2, upstream.calls("a.example")) + } + + @Test + fun `concurrent lookups for the same host coalesce to one upstream call`() { + val gated = GatedDns(mapOf("hot.example" to listOf(ip("9.9.9.9")))) + val dns = AmethystDns(delegate = gated) + val pool = Executors.newFixedThreadPool(8) + + try { + val results = (1..8).map { pool.submit> { dns.lookup("hot.example") } } + assertTrue( + "Leader should have started the upstream lookup", + gated.started.await(2, TimeUnit.SECONDS), + ) + gated.release.countDown() + + results.forEach { + assertEquals(listOf(ip("9.9.9.9")), it.get(2, TimeUnit.SECONDS)) + } + assertEquals(1, gated.calls.get()) + } finally { + pool.shutdownNow() + } + } + + @Test + fun `lookups for different hosts run in parallel`() { + val responses = mapOf("a" to listOf(ip("1.1.1.1")), "b" to listOf(ip("2.2.2.2"))) + val parallelism = AtomicInteger() + val peak = AtomicInteger() + val release = CountDownLatch(1) + + val instrumented = + Dns { hostname -> + val now = parallelism.incrementAndGet() + peak.updateAndGet { maxOf(it, now) } + try { + release.await(2, TimeUnit.SECONDS) + responses[hostname] ?: throw UnknownHostException(hostname) + } finally { + parallelism.decrementAndGet() + } + } + val dns = AmethystDns(delegate = instrumented) + val pool = Executors.newFixedThreadPool(2) + + try { + val futureA = pool.submit> { dns.lookup("a") } + val futureB = pool.submit> { dns.lookup("b") } + + // Wait briefly for both threads to enter the resolver, then let them out. + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2) + while (peak.get() < 2 && System.nanoTime() < deadline) { + Thread.sleep(5) + } + release.countDown() + + futureA.get(2, TimeUnit.SECONDS) + futureB.get(2, TimeUnit.SECONDS) + + assertEquals("Both hosts should resolve concurrently", 2, peak.get()) + } finally { + pool.shutdownNow() + } + } + + @Test + fun `invalidate clears cache so next lookup hits upstream`() { + val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("a.example") + dns.invalidate() + dns.lookup("a.example") + + assertEquals(2, upstream.calls("a.example")) + } + + @Test + fun `invalidate by host removes only that entry`() { + val upstream = + CountingDns( + mapOf( + "a.example" to listOf(ip("1.2.3.4")), + "b.example" to listOf(ip("5.6.7.8")), + ), + ) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("a.example") + dns.lookup("b.example") + dns.invalidate("a.example") + dns.lookup("a.example") + dns.lookup("b.example") + + assertEquals(2, upstream.calls("a.example")) + assertEquals(1, upstream.calls("b.example")) + } +} From 95a34271150589020dcd14ea6742f2eba601648d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:16:18 +0000 Subject: [PATCH 02/11] chore(okhttp): bump AmethystDns cache to 2000 entries A Nostr feed can touch hundreds of distinct hosts (Blossom servers, relays, NIP-05 domains, image proxies). The 256-entry cap was small enough that active users would churn the LRU and pay extra getaddrinfo calls. 2000 still costs <100KB of heap. --- .../com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index a453864a5e..1e044ff4d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -49,7 +49,7 @@ import java.util.concurrent.TimeUnit */ class AmethystDns( private val delegate: Dns = Dns.SYSTEM, - private val maxEntries: Int = 256, + private val maxEntries: Int = 2000, positiveTtlMs: Long = TimeUnit.MINUTES.toMillis(5), negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), ) : Dns { From 71bf1fd29024371285a8d0a2be4d2beaf715dbaa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:32:40 +0000 Subject: [PATCH 03/11] perf(okhttp): lock-free DNS cache + 24h positive TTL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous cache was a synchronizedMap(LinkedHashMap(access-order=true)). Access-order LRU rewrites the linked list on get(), so every cache hit took the global monitor — at 700 concurrent relay reconnects, the lock serialized every dispatcher worker through a single critical section. Switch to ConcurrentHashMap so reads are lock-free. Amethyst's steady state is well under maxEntries (~750 distinct hosts vs cap of 2000), so strict LRU was never going to evict anything and the access-order machinery was pure contention. Bump positive TTL from 5min to 24h: relay and CDN IPs change on the order of days, and we are not a recursive resolver — there's no correctness reason to honor authoritative TTLs. A 5min cycle made us re-resolve all 700 relays every 5 minutes. Also add a leader-side cache re-check after acquiring inflight ownership. If a peer leader refreshed the entry while we were claiming the slot, we skip getaddrinfo entirely. Eviction is now a cheap on-demand sweep of expired entries when the map exceeds maxEntries; in normal use it never runs. --- .../amethyst/service/okhttp/AmethystDns.kt | 76 ++++++++++++------- 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index 1e044ff4d6..7648ba37d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.amethyst.service.okhttp import okhttp3.Dns import java.net.InetAddress import java.net.UnknownHostException -import java.util.Collections import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ExecutionException @@ -32,36 +31,42 @@ import java.util.concurrent.TimeUnit /** * Concurrent, caching DNS resolver for OkHttp. * - * The system resolver call ([InetAddress.getAllByName] used by [Dns.SYSTEM]) is a blocking JNI - * hop into `getaddrinfo`. On a busy feed we may issue dozens of HTTP calls to a handful of hosts - * in the same second; the default behaviour pays the resolver tax once per call and serializes - * the OkHttp dispatcher worker that asked for it. + * Tuned for an Amethyst-shaped workload: ~700 relays plus a small set of media/profile/NIP-05 + * hosts that reappear constantly. Steady state is well under [maxEntries] distinct hosts whose + * IPs change on the order of days, so we want to resolve each one once per session and never + * touch DNS again. * - * This resolver adds three things on top of the system resolver: + * Properties: * - * 1. An LRU + TTL cache, so repeated lookups of the same host short-circuit before touching the - * network. Negative results get a short TTL so a typo doesn't keep hammering DNS. - * 2. Single-flight coalescing: when N OkHttp threads ask for the same host concurrently, only - * one of them performs the upstream lookup. The others block on the same future and pick up - * the result. Without this, ten parallel image requests to the same CDN make ten DNS calls. - * 3. No global lock on the slow path: lookups for *different* hosts proceed in parallel because - * the upstream resolver is invoked outside any monitor. + * 1. **Lock-free reads.** Cache is a [ConcurrentHashMap], so the hot path (cache hit) does no + * locking. The previous incarnation used `synchronizedMap(LinkedHashMap(access-order=true))`, + * which turned every `get` into a monitor-protected write because access-order LRU rewrites + * the linked list on read — at 700 concurrent relay reconnects, the lock dominated. + * 2. **Single-flight coalescing.** N concurrent OkHttp threads asking for the same host share + * one upstream `getaddrinfo`. The leader resolves; followers block on the same future. If a + * peer leader refreshed the entry while we were claiming leadership, the leader re-checks + * the cache and skips the system call entirely. + * 3. **Generous positive TTL.** Defaults to 24h. Relay and CDN IPs almost never move, and we + * are not a recursive resolver — there is no correctness reason to honor authoritative TTLs. + * Pair with [invalidate] on connection failures or network changes to recover from real + * IP moves. + * 4. **Short negative TTL.** Failed lookups are remembered for 10s so a typo or a transiently + * down host doesn't keep paying for `getaddrinfo`, but a real outage recovers quickly. + * + * Remaining blocking points are unavoidable: the leader's [Dns.lookup] call is a synchronous JNI + * hop into the system resolver, and followers must wait on the leader's future. Both are + * bypassed entirely on cache hit, which is the steady state. */ class AmethystDns( private val delegate: Dns = Dns.SYSTEM, private val maxEntries: Int = 2000, - positiveTtlMs: Long = TimeUnit.MINUTES.toMillis(5), + positiveTtlMs: Long = TimeUnit.HOURS.toMillis(24), negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), ) : Dns { private val positiveTtlNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlMs) private val negativeTtlNanos = TimeUnit.MILLISECONDS.toNanos(negativeTtlMs) - private val cache: MutableMap = - Collections.synchronizedMap( - object : LinkedHashMap(64, 0.75f, true) { - override fun removeEldestEntry(eldest: Map.Entry): Boolean = size > maxEntries - }, - ) + private val cache = ConcurrentHashMap() private val inflight = ConcurrentHashMap>>() override fun lookup(hostname: String): List { @@ -85,14 +90,22 @@ class AmethystDns( future: CompletableFuture>, ): List { try { - val addresses = delegate.lookup(hostname) - put(hostname, addresses, positiveTtlNanos) + // Re-check after claiming leadership: a peer may have refreshed the cache between + // our miss and our putIfAbsent. Skips a duplicate getaddrinfo in that race. + val cached = cache[hostname] + val addresses = + if (cached != null && cached.expiresAtNanos > System.nanoTime()) { + cached.unwrap(hostname) + } else { + try { + delegate.lookup(hostname).also { put(hostname, it, positiveTtlNanos) } + } catch (e: UnknownHostException) { + put(hostname, emptyList(), negativeTtlNanos) + throw e + } + } future.complete(addresses) return addresses - } catch (e: UnknownHostException) { - put(hostname, emptyList(), negativeTtlNanos) - future.completeExceptionally(e) - throw e } catch (e: Throwable) { future.completeExceptionally(e) throw e @@ -126,6 +139,15 @@ class AmethystDns( ttlNanos: Long, ) { cache[hostname] = Entry(addresses, System.nanoTime() + ttlNanos) + if (cache.size > maxEntries) evictExpired() + } + + private fun evictExpired() { + val now = System.nanoTime() + val it = cache.entries.iterator() + while (it.hasNext()) { + if (it.next().value.expiresAtNanos <= now) it.remove() + } } /** Drop all cached entries. Call when the network changes (e.g. WiFi <-> mobile). */ @@ -133,7 +155,7 @@ class AmethystDns( cache.clear() } - /** Drop a single host's cached entry. */ + /** Drop a single host's cached entry. Call when a connection to it fails. */ fun invalidate(hostname: String) { cache.remove(hostname) } From feef50985d4298004ea675dde4887e175cdb6cb8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:40:52 +0000 Subject: [PATCH 04/11] perf(okhttp): stale-while-revalidate + TTL jitter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the first lookup of a host, recurring connections never block on getaddrinfo again. Soft-expired positive entries are returned immediately while a background refresh updates the cache through a small fixed thread pool (8 daemon threads). The refresh is coalesced through the same inflight map, so a fan-out of stale reads to the same host still triggers exactly one upstream call. Negative entries deliberately do NOT serve stale — a transient DNS failure must recover quickly, not keep returning UnknownHostException. A failed refresh demotes the entry to negative so the next caller sees a fresh failure rather than forever-stale wrong IPs. Add TTL jitter on positive writes so a burst of co-written entries (e.g. ~700 relay reconnects at app start) doesn't all expire at the same instant 24h later. Default jitter equals the base TTL, so entries spread their expiries uniformly across [24h, 48h]. Combined with the bounded refresh executor, even a daily-app-open user with 1000 stale hosts generates a few seconds of background work and zero blocking waits in the foreground. --- .../amethyst/service/okhttp/AmethystDns.kt | 111 ++++++++++++---- .../service/okhttp/AmethystDnsTest.kt | 119 +++++++++++++++++- 2 files changed, 201 insertions(+), 29 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index 7648ba37d6..22797293ef 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -26,44 +26,55 @@ import java.net.UnknownHostException import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ExecutionException +import java.util.concurrent.Executor +import java.util.concurrent.Executors +import java.util.concurrent.RejectedExecutionException +import java.util.concurrent.ThreadLocalRandom import java.util.concurrent.TimeUnit /** - * Concurrent, caching DNS resolver for OkHttp. + * Concurrent, caching, stale-while-revalidate DNS resolver for OkHttp. * * Tuned for an Amethyst-shaped workload: ~700 relays plus a small set of media/profile/NIP-05 - * hosts that reappear constantly. Steady state is well under [maxEntries] distinct hosts whose - * IPs change on the order of days, so we want to resolve each one once per session and never - * touch DNS again. + * hosts that reappear constantly, whose IPs change on the order of days. * * Properties: * - * 1. **Lock-free reads.** Cache is a [ConcurrentHashMap], so the hot path (cache hit) does no - * locking. The previous incarnation used `synchronizedMap(LinkedHashMap(access-order=true))`, - * which turned every `get` into a monitor-protected write because access-order LRU rewrites - * the linked list on read — at 700 concurrent relay reconnects, the lock dominated. - * 2. **Single-flight coalescing.** N concurrent OkHttp threads asking for the same host share - * one upstream `getaddrinfo`. The leader resolves; followers block on the same future. If a - * peer leader refreshed the entry while we were claiming leadership, the leader re-checks - * the cache and skips the system call entirely. - * 3. **Generous positive TTL.** Defaults to 24h. Relay and CDN IPs almost never move, and we - * are not a recursive resolver — there is no correctness reason to honor authoritative TTLs. - * Pair with [invalidate] on connection failures or network changes to recover from real - * IP moves. - * 4. **Short negative TTL.** Failed lookups are remembered for 10s so a typo or a transiently - * down host doesn't keep paying for `getaddrinfo`, but a real outage recovers quickly. + * 1. **Lock-free reads.** Cache is a [ConcurrentHashMap]. The hot path (cache hit) takes no + * locks, so 700 concurrent relay reconnects fan out across OkHttp dispatcher threads + * instead of serializing through one monitor. + * 2. **Single-flight coalescing.** N concurrent threads asking for the same host share one + * upstream `getaddrinfo`. The leader resolves; followers block on the same future. + * 3. **Stale-while-revalidate.** Once a host has been resolved, recurring lookups *never* + * block on `getaddrinfo` again. After the soft TTL expires, we return the previous answer + * immediately and kick a background refresh. The refresh is coalesced through the same + * `inflight` map, so a burst of stale reads triggers one refresh per host. If the refresh + * fails (`UnknownHostException`), the cache entry is demoted to negative, so the next + * caller gets a fresh failure rather than forever-wrong stale IPs. + * 4. **Negative entries do *not* serve stale.** When an `UnknownHostException` cache entry + * expires, the next call goes through the synchronous path. We want transient failures to + * recover quickly, not keep returning stale failures. + * 5. **Generous positive TTL with jitter.** Defaults to 24h plus up to 24h of random jitter, + * so each entry expires somewhere in [base, base + jitter]. This breaks the synchronized + * herd that would otherwise form when ~700 relay reconnects all populate the cache in the + * same second: their expiries spread across a 24h window instead of landing at the same + * instant 24h later. A user who opens the app once a day catches only a small fraction of + * entries stale per session, and refreshes drip through the executor pool naturally. * - * Remaining blocking points are unavoidable: the leader's [Dns.lookup] call is a synchronous JNI - * hop into the system resolver, and followers must wait on the leader's future. Both are - * bypassed entirely on cache hit, which is the steady state. + * Remaining blocking points: the very first lookup of a host blocks on `getaddrinfo` + * (unavoidable — there's nothing to serve stale yet), and followers waiting on that first + * lookup block on `future.get()`. Background refreshes never block any caller. */ class AmethystDns( private val delegate: Dns = Dns.SYSTEM, private val maxEntries: Int = 2000, positiveTtlMs: Long = TimeUnit.HOURS.toMillis(24), + positiveTtlJitterMs: Long = TimeUnit.HOURS.toMillis(24), negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), + private val refreshExecutor: Executor = DEFAULT_REFRESH_EXECUTOR, ) : Dns { private val positiveTtlNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlMs) + private val positiveTtlJitterNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlJitterMs) private val negativeTtlNanos = TimeUnit.MILLISECONDS.toNanos(negativeTtlMs) private val cache = ConcurrentHashMap() @@ -71,9 +82,16 @@ class AmethystDns( override fun lookup(hostname: String): List { cache[hostname]?.let { entry -> - if (entry.expiresAtNanos > System.nanoTime()) { + val now = System.nanoTime() + if (entry.expiresAtNanos > now) { return entry.unwrap(hostname) } + // Soft-expired positive entry: serve stale, refresh in background. Negative + // entries fall through to the sync path so transient failures recover quickly. + if (entry.addresses.isNotEmpty()) { + triggerBackgroundRefresh(hostname) + return entry.addresses + } } val newFuture = CompletableFuture>() @@ -85,6 +103,24 @@ class AmethystDns( } } + private fun triggerBackgroundRefresh(hostname: String) { + val refreshFuture = CompletableFuture>() + // Coalesce: if a sync lookup or a prior refresh is already in flight, skip. + if (inflight.putIfAbsent(hostname, refreshFuture) != null) return + try { + refreshExecutor.execute { + try { + resolveAsLeader(hostname, refreshFuture) + } catch (_: Throwable) { + // Caller already got the stale answer; the failure is recorded on the + // future and (for UnknownHostException) demoted in the cache. + } + } + } catch (_: RejectedExecutionException) { + inflight.remove(hostname, refreshFuture) + } + } + private fun resolveAsLeader( hostname: String, future: CompletableFuture>, @@ -98,9 +134,9 @@ class AmethystDns( cached.unwrap(hostname) } else { try { - delegate.lookup(hostname).also { put(hostname, it, positiveTtlNanos) } + delegate.lookup(hostname).also { putPositive(hostname, it) } } catch (e: UnknownHostException) { - put(hostname, emptyList(), negativeTtlNanos) + putNegative(hostname) throw e } } @@ -133,12 +169,24 @@ class AmethystDns( } } - private fun put( + private fun putPositive( hostname: String, addresses: List, - ttlNanos: Long, ) { - cache[hostname] = Entry(addresses, System.nanoTime() + ttlNanos) + // Jitter the expiry so a burst of co-written entries (e.g. ~700 relay reconnects at app + // start) doesn't all expire at the same instant 24h later. + val jitter = + if (positiveTtlJitterNanos > 0L) { + ThreadLocalRandom.current().nextLong(positiveTtlJitterNanos) + } else { + 0L + } + cache[hostname] = Entry(addresses, System.nanoTime() + positiveTtlNanos + jitter) + if (cache.size > maxEntries) evictExpired() + } + + private fun putNegative(hostname: String) { + cache[hostname] = Entry(emptyList(), System.nanoTime() + negativeTtlNanos) if (cache.size > maxEntries) evictExpired() } @@ -168,6 +216,15 @@ class AmethystDns( } companion object { + // Small fixed pool of daemon threads. Refreshes block on getaddrinfo (~tens of ms), + // so a handful of threads is plenty even when many hosts go stale at once — extra + // refreshes queue up without blocking any caller, since callers always get the + // stale answer instantly. + private val DEFAULT_REFRESH_EXECUTOR: Executor = + Executors.newFixedThreadPool(8) { r -> + Thread(r, "amethyst-dns-refresh").apply { isDaemon = true } + } + /** * Process-wide instance shared by every OkHttp client built in the app, so a host resolved * for an image fetch is reused when a relay handshake or NIP-05 lookup hits the same host. diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt index 0c0b88554a..91d25e2572 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -29,9 +29,11 @@ import org.junit.Test import java.net.InetAddress import java.net.UnknownHostException import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executor import java.util.concurrent.Executors import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference class AmethystDnsTest { private fun ip(value: String) = InetAddress.getByName(value) @@ -89,22 +91,135 @@ class AmethystDnsTest { } @Test - fun `positive entries expire`() { + fun `expired positive entry serves stale and refreshes in background`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val syncRefresh = Executor { it.run() } val dns = AmethystDns( delegate = upstream, positiveTtlMs = 1, + positiveTtlJitterMs = 0, negativeTtlMs = 1, + refreshExecutor = syncRefresh, ) dns.lookup("a.example") Thread.sleep(20) - dns.lookup("a.example") + // Returns the stale cached value AND triggers a refresh on the synchronous executor. + assertEquals(listOf(ip("1.2.3.4")), dns.lookup("a.example")) assertEquals(2, upstream.calls("a.example")) } + @Test + fun `expired negative entry does not stale-while-revalidate`() { + val upstream = CountingDns(emptyMap()) + val dns = + AmethystDns( + delegate = upstream, + positiveTtlJitterMs = 0, + negativeTtlMs = 1, + ) + + assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } + Thread.sleep(20) + assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } + + // Two synchronous calls — failed lookups must retry quickly, not be served stale. + assertEquals(2, upstream.calls("missing.example")) + } + + @Test + fun `stale read returns previous IP while refresh updates the cache`() { + val responses = AtomicReference>(listOf(ip("1.2.3.4"))) + val calls = AtomicInteger() + val upstream = + Dns { _ -> + calls.incrementAndGet() + responses.get() + } + val syncRefresh = Executor { it.run() } + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 1, + positiveTtlJitterMs = 0, + refreshExecutor = syncRefresh, + ) + + assertEquals(listOf(ip("1.2.3.4")), dns.lookup("a.example")) + Thread.sleep(20) + + // Upstream now returns a new IP. The next lookup should still serve the OLD IP + // immediately, while the synchronous executor performs the refresh inline. + responses.set(listOf(ip("5.6.7.8"))) + val stale = dns.lookup("a.example") + assertEquals(listOf(ip("1.2.3.4")), stale) + assertEquals(2, calls.get()) + + // Cache now holds the refreshed IP — no further upstream calls. + assertEquals(listOf(ip("5.6.7.8")), dns.lookup("a.example")) + assertEquals(2, calls.get()) + } + + @Test + fun `stale burst on the same host triggers a single refresh`() { + val gated = GatedDns(mapOf("hot.example" to listOf(ip("9.9.9.9")))) + // Pre-populate via a separate, non-gated upstream then swap in the gated one for + // the refresh — easiest way: bootstrap by writing through a delegate that completes + // immediately, then attach the gate to count parallel refresh calls. + val bootstrapCalls = AtomicInteger() + val dynamic = + object : Dns { + @Volatile var useGated = false + + override fun lookup(hostname: String): List = + if (useGated) { + gated.lookup(hostname) + } else { + bootstrapCalls.incrementAndGet() + listOf(ip("9.9.9.9")) + } + } + val pool = Executors.newFixedThreadPool(8) + val dns = + AmethystDns( + delegate = dynamic, + positiveTtlMs = 1, + positiveTtlJitterMs = 0, + refreshExecutor = pool, + ) + try { + dns.lookup("hot.example") + assertEquals(1, bootstrapCalls.get()) + Thread.sleep(20) + dynamic.useGated = true + + // Fan out 16 stale reads concurrently. They all return the stale answer + // immediately; only one refresh should be queued/executing. + val callerPool = Executors.newFixedThreadPool(16) + try { + val results = + (1..16).map { + callerPool.submit> { dns.lookup("hot.example") } + } + results.forEach { assertEquals(listOf(ip("9.9.9.9")), it.get(2, TimeUnit.SECONDS)) } + assertTrue( + "Refresh should have started", + gated.started.await(2, TimeUnit.SECONDS), + ) + gated.release.countDown() + // Allow refresh to complete. + Thread.sleep(100) + assertEquals("Only one refresh upstream call", 1, gated.calls.get()) + } finally { + callerPool.shutdownNow() + } + } finally { + pool.shutdownNow() + } + } + @Test fun `concurrent lookups for the same host coalesce to one upstream call`() { val gated = GatedDns(mapOf("hot.example" to listOf(ip("9.9.9.9")))) From 893cc249fbdf6c99b65de9ad90eaa6d597581ff2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 17:51:06 +0000 Subject: [PATCH 05/11] feat(okhttp): persist DNS cache across process restarts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold starts were the resolver's worst case: every host paid a sync getaddrinfo. With a persisted snapshot, every previously-seen host falls into the stale-while-revalidate path on first lookup — the cached IP is served immediately and a background refresh updates it. ~700 blocking system calls at app start become zero. Switch entry expiries from System.nanoTime to System.currentTimeMillis so timestamps survive process death (nanoTime is monotonic per process, undefined across restarts). Add snapshot()/restore() that serialize only fresh positive entries — negative entries are skipped and re-resolved synchronously, and restore() uses putIfAbsent so a fresh in-memory entry is never clobbered by a stale on-disk one. Add AmethystDnsStore as a thin SharedPreferences + Jackson wrapper. The blob is plain (not encrypted): hostnames are already exposed in the user's signed relay list, Coil's image cache, and the system resolver's own state. Wire load + save into AppModules: - load() runs once at app start on the IO scope. - save() runs every 5 minutes (skipped when nothing has changed via the dirty flag), on trim() when the app backgrounds, and once on terminate() as a best-effort flush. --- .../com/vitorpamplona/amethyst/AppModules.kt | 27 +++++ .../amethyst/service/okhttp/AmethystDns.kt | 98 +++++++++++++++---- .../service/okhttp/AmethystDnsStore.kt | 97 ++++++++++++++++++ .../service/okhttp/AmethystDnsTest.kt | 94 ++++++++++++++++++ 4 files changed, 299 insertions(+), 17 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index d536908bdd..af11cf8115 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -53,6 +53,7 @@ import com.vitorpamplona.amethyst.service.location.LocationState import com.vitorpamplona.amethyst.service.notifications.AlwaysOnNotificationServiceManager import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher import com.vitorpamplona.amethyst.service.notifications.PokeyReceiver +import com.vitorpamplona.amethyst.service.okhttp.AmethystDnsStore import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManager import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManagerForRelays import com.vitorpamplona.amethyst.service.okhttp.EncryptionKeyCache @@ -201,6 +202,11 @@ class AppModules( // Key cache service to download and decrypt encrypted files before caching them. val keyCache = EncryptionKeyCache() + // Persists the shared DNS resolver's positive cache across process restarts so cold starts + // don't pay ~700 sync getaddrinfo calls. Restored entries fall through to the + // stale-while-revalidate path on first lookup. + val dnsStore = AmethystDnsStore(appContext) + // manages all the other connections separately from relays. val okHttpClients = DualHttpClientManager( @@ -501,6 +507,22 @@ class AppModules( fun initiate(appContext: Context) { Thread.setDefaultUncaughtExceptionHandler(UnexpectedCrashSaver(crashReportCache, applicationIOScope)) + // Restore the persisted DNS cache before any networking starts. Lookups that fire + // before this completes fall through to the sync resolver path (existing behavior); + // once restored, every previously-seen host hits the stale-while-revalidate path + // instead of blocking on getaddrinfo. + applicationIOScope.launch { + dnsStore.load() + } + + // Periodically flush the DNS cache. Saves are skipped when nothing has changed. + applicationIOScope.launch { + while (true) { + delay(5 * 60 * 1000L) + dnsStore.save() + } + } + applicationIOScope.launch { // loads main account quickly. LocalPreferences.loadAccountConfigFromEncryptedStorage() @@ -565,12 +587,17 @@ class AppModules( BackgroundMedia.removeBackgroundControllerAndReleaseIt() PlaybackServiceClient.shutdown() alwaysOnNotificationServiceManager.stop() + // Best-effort flush before the scope is cancelled. Android rarely calls onTerminate in + // production, but when it does we get one last chance to persist the cache. + runCatching { dnsStore.save() } applicationIOScope.cancel("Application onTerminate $appContext") accountsCache.clear() } fun trim() { applicationIOScope.launch { + // Backgrounding is a natural moment to flush the DNS cache. + dnsStore.save() val loggedIn = accountsCache.accounts.value.values trimmingService.run(loggedIn, LocalPreferences.allSavedAccounts()) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index 22797293ef..e67d4c638b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -31,6 +31,7 @@ import java.util.concurrent.Executors import java.util.concurrent.RejectedExecutionException import java.util.concurrent.ThreadLocalRandom import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean /** * Concurrent, caching, stale-while-revalidate DNS resolver for OkHttp. @@ -57,9 +58,11 @@ import java.util.concurrent.TimeUnit * 5. **Generous positive TTL with jitter.** Defaults to 24h plus up to 24h of random jitter, * so each entry expires somewhere in [base, base + jitter]. This breaks the synchronized * herd that would otherwise form when ~700 relay reconnects all populate the cache in the - * same second: their expiries spread across a 24h window instead of landing at the same - * instant 24h later. A user who opens the app once a day catches only a small fraction of - * entries stale per session, and refreshes drip through the executor pool naturally. + * same second. + * 6. **Persistable across restarts.** [snapshot] and [restore] expose the positive cache as a + * plain data list so a companion store can survive process death. Wall-clock millis are + * used for expiries (not [System.nanoTime], which resets per process) so a restored entry + * keeps its remaining lifetime. * * Remaining blocking points: the very first lookup of a host blocks on `getaddrinfo` * (unavoidable — there's nothing to serve stale yet), and followers waiting on that first @@ -73,17 +76,18 @@ class AmethystDns( negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), private val refreshExecutor: Executor = DEFAULT_REFRESH_EXECUTOR, ) : Dns { - private val positiveTtlNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlMs) - private val positiveTtlJitterNanos = TimeUnit.MILLISECONDS.toNanos(positiveTtlJitterMs) - private val negativeTtlNanos = TimeUnit.MILLISECONDS.toNanos(negativeTtlMs) + private val positiveTtlMillis = positiveTtlMs + private val positiveTtlJitterMillis = positiveTtlJitterMs + private val negativeTtlMillis = negativeTtlMs private val cache = ConcurrentHashMap() private val inflight = ConcurrentHashMap>>() + private val dirty = AtomicBoolean(false) override fun lookup(hostname: String): List { cache[hostname]?.let { entry -> - val now = System.nanoTime() - if (entry.expiresAtNanos > now) { + val now = System.currentTimeMillis() + if (entry.expiresAtMillis > now) { return entry.unwrap(hostname) } // Soft-expired positive entry: serve stale, refresh in background. Negative @@ -130,7 +134,7 @@ class AmethystDns( // our miss and our putIfAbsent. Skips a duplicate getaddrinfo in that race. val cached = cache[hostname] val addresses = - if (cached != null && cached.expiresAtNanos > System.nanoTime()) { + if (cached != null && cached.expiresAtMillis > System.currentTimeMillis()) { cached.unwrap(hostname) } else { try { @@ -176,41 +180,91 @@ class AmethystDns( // Jitter the expiry so a burst of co-written entries (e.g. ~700 relay reconnects at app // start) doesn't all expire at the same instant 24h later. val jitter = - if (positiveTtlJitterNanos > 0L) { - ThreadLocalRandom.current().nextLong(positiveTtlJitterNanos) + if (positiveTtlJitterMillis > 0L) { + ThreadLocalRandom.current().nextLong(positiveTtlJitterMillis) } else { 0L } - cache[hostname] = Entry(addresses, System.nanoTime() + positiveTtlNanos + jitter) + cache[hostname] = Entry(addresses, System.currentTimeMillis() + positiveTtlMillis + jitter) + dirty.set(true) if (cache.size > maxEntries) evictExpired() } private fun putNegative(hostname: String) { - cache[hostname] = Entry(emptyList(), System.nanoTime() + negativeTtlNanos) + cache[hostname] = Entry(emptyList(), System.currentTimeMillis() + negativeTtlMillis) + // Negative entries are never persisted, so they don't dirty the cache. if (cache.size > maxEntries) evictExpired() } private fun evictExpired() { - val now = System.nanoTime() + val now = System.currentTimeMillis() val it = cache.entries.iterator() while (it.hasNext()) { - if (it.next().value.expiresAtNanos <= now) it.remove() + if (it.next().value.expiresAtMillis <= now) it.remove() } } /** Drop all cached entries. Call when the network changes (e.g. WiFi <-> mobile). */ fun invalidate() { cache.clear() + dirty.set(true) } /** Drop a single host's cached entry. Call when a connection to it fails. */ fun invalidate(hostname: String) { - cache.remove(hostname) + if (cache.remove(hostname) != null) dirty.set(true) + } + + /** + * Snapshot of the positive cache for persistence. Negative entries and expired entries are + * omitted. The expiry timestamps are wall-clock millis (epoch), so they remain meaningful + * across process restarts. + */ + fun snapshot(): List { + val now = System.currentTimeMillis() + val out = ArrayList(cache.size) + for ((host, entry) in cache) { + if (entry.addresses.isEmpty()) continue + if (entry.expiresAtMillis <= now) continue + val ips = entry.addresses.mapNotNull { it.hostAddress } + if (ips.isNotEmpty()) { + out += DnsCacheRecord(host, ips, entry.expiresAtMillis) + } + } + return out + } + + /** + * Restore from a previously taken snapshot. Entries already expired on disk are dropped. + * Existing in-memory entries are NOT overwritten (so a fresh lookup that completes before + * restore lands keeps its newer answer). + */ + fun restore(records: List) { + val now = System.currentTimeMillis() + for (record in records) { + if (record.expiresAtMillis <= now) continue + val addresses = + record.addresses.mapNotNull { literal -> + // getByName on a numeric literal parses without doing DNS. + runCatching { InetAddress.getByName(literal) }.getOrNull() + } + if (addresses.isNotEmpty()) { + cache.putIfAbsent(record.hostname, Entry(addresses, record.expiresAtMillis)) + } + } + } + + /** True if the cache has changed since the last [clearDirty]. */ + fun isDirty(): Boolean = dirty.get() + + /** Marks the cache clean. Call after a successful persist. */ + fun clearDirty() { + dirty.set(false) } private class Entry( val addresses: List, - val expiresAtNanos: Long, + val expiresAtMillis: Long, ) { fun unwrap(hostname: String): List = addresses.ifEmpty { throw UnknownHostException(hostname) } } @@ -232,3 +286,13 @@ class AmethystDns( val shared: AmethystDns by lazy { AmethystDns() } } } + +/** Persistable record. Public so [AmethystDnsStore] can serialize it via Jackson. */ +data class DnsCacheRecord( + val hostname: String, + val addresses: List, + val expiresAtMillis: Long, +) { + // No-arg constructor for Jackson. + constructor() : this("", emptyList(), 0L) +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt new file mode 100644 index 0000000000..b9424b2ed3 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.okhttp + +import android.content.Context +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue +import com.vitorpamplona.quartz.utils.Log + +/** + * Persists [AmethystDns]'s positive cache to a small `SharedPreferences` blob so the resolver + * starts hot after a process restart. + * + * Cold starts are the resolver's worst case — every host is a sync `getaddrinfo`. With a + * persisted snapshot, every previously-seen host falls into the stale-while-revalidate path on + * first lookup: the cached IP is served immediately, and a background refresh updates it. That + * turns ~700 blocking system calls at app start into zero. + * + * The blob is plain (not encrypted) — hostnames are already exposed in the user's signed relay + * list, Coil's image cache, and the system resolver's own state. ~700 entries × ~80 bytes ≈ + * ~55 KB of JSON. + */ +class AmethystDnsStore( + private val context: Context, + private val dns: AmethystDns = AmethystDns.shared, +) { + private val prefs by lazy { context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) } + + /** + * Read the persisted snapshot and merge it into the resolver. Existing in-memory entries + * are preserved (see [AmethystDns.restore]). Safe to call once at app start. Blocking I/O — + * call from a background thread. + */ + fun load() { + val json = prefs.getString(KEY_CACHE, null) ?: return + val records = + try { + MAPPER.readValue>(json) + } catch (t: Throwable) { + Log.w(TAG) { "Dropping corrupt DNS cache blob: ${t.message}" } + prefs.edit().remove(KEY_CACHE).apply() + return + } + dns.restore(records) + // Restoring entries that already existed in memory is a no-op, but the act of loading + // shouldn't mark the cache dirty. + dns.clearDirty() + Log.d(TAG) { "Restored ${records.size} DNS cache entries" } + } + + /** + * Write the current snapshot to disk if the cache has changed since the last save. Blocking + * I/O — call from a background thread. + */ + fun save() { + if (!dns.isDirty()) return + val records = dns.snapshot() + try { + val json = MAPPER.writeValueAsString(records) + prefs.edit().putString(KEY_CACHE, json).apply() + dns.clearDirty() + Log.d(TAG) { "Persisted ${records.size} DNS cache entries" } + } catch (t: Throwable) { + Log.w(TAG) { "Failed to persist DNS cache: ${t.message}" } + } + } + + /** Force-clear the on-disk cache. Useful for diagnostics or when the user wipes data. */ + fun clear() { + prefs.edit().remove(KEY_CACHE).apply() + } + + companion object { + private const val TAG = "AmethystDnsStore" + private const val PREFS_NAME = "amethyst_dns_cache" + private const val KEY_CACHE = "dns_cache_v1" + private val MAPPER = jacksonObjectMapper() + } +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt index 91d25e2572..fcdcbfb733 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.service.okhttp import okhttp3.Dns import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertSame import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue @@ -316,4 +317,97 @@ class AmethystDnsTest { assertEquals(2, upstream.calls("a.example")) assertEquals(1, upstream.calls("b.example")) } + + @Test + fun `snapshot includes only fresh positive entries`() { + val upstream = + CountingDns( + mapOf( + "live.example" to listOf(ip("1.2.3.4")), + "missing.example" to emptyList(), + ), + ) + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 60_000, + positiveTtlJitterMs = 0, + ) + dns.lookup("live.example") + runCatching { dns.lookup("missing.example") } + + val snapshot = dns.snapshot() + assertEquals(1, snapshot.size) + assertEquals("live.example", snapshot[0].hostname) + assertEquals(listOf("1.2.3.4"), snapshot[0].addresses) + } + + @Test + fun `restore replays cached entries without hitting upstream`() { + val upstream = CountingDns(mapOf("relay.example" to listOf(ip("9.9.9.9")))) + val dns = AmethystDns(delegate = upstream) + + val expiresAt = System.currentTimeMillis() + 60_000 + dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiresAt))) + + assertEquals(listOf(ip("9.9.9.9")), dns.lookup("relay.example")) + assertEquals("Restored entry should serve without upstream", 0, upstream.calls("relay.example")) + } + + @Test + fun `restore drops entries already expired on disk`() { + val upstream = CountingDns(mapOf("relay.example" to listOf(ip("9.9.9.9")))) + val dns = AmethystDns(delegate = upstream) + + val expiredAt = System.currentTimeMillis() - 1_000 + dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiredAt))) + + dns.lookup("relay.example") + assertEquals(1, upstream.calls("relay.example")) + } + + @Test + fun `restore does not overwrite a fresh in-memory entry`() { + val upstream = CountingDns(mapOf("relay.example" to listOf(ip("1.1.1.1")))) + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 60_000, + positiveTtlJitterMs = 0, + ) + + // Live lookup populates with 1.1.1.1. + dns.lookup("relay.example") + // Then a (stale-on-disk-but-not-yet-expired) restore arrives with a different IP. + dns.restore( + listOf( + DnsCacheRecord( + "relay.example", + listOf("9.9.9.9"), + System.currentTimeMillis() + 60_000, + ), + ), + ) + + assertEquals("In-memory entry wins", listOf(ip("1.1.1.1")), dns.lookup("relay.example")) + } + + @Test + fun `dirty flag tracks positive writes`() { + val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 60_000, + positiveTtlJitterMs = 0, + ) + + assertFalse("Fresh resolver is not dirty", dns.isDirty()) + dns.lookup("a.example") + assertTrue("First positive write dirties cache", dns.isDirty()) + + dns.clearDirty() + dns.lookup("a.example") // cache hit, no write + assertFalse("Cache hit does not dirty", dns.isDirty()) + } } From e4897126a1a8417ea19d07aaccc2bf0765c586af Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 19:22:04 +0000 Subject: [PATCH 06/11] fix(okhttp): canonicalize DNS hosts and invalidate on call failure Two follow-ups from the resolver audit: 1. Lowercase the hostname at the public boundary (lookup, invalidate, restore). DNS is case-insensitive; OkHttp normally hands us lowercase but custom callers and persisted records can mix case. This prevents "Example.com" and "example.com" from creating separate cache entries. 2. Drop the cache entry when an OkHttp call to a host fails outright. Without this, a stale-cached IP that no longer works would be served for up to 24h before the soft TTL ran out. Hooking callFailed (final-stage signal after OkHttp tried every address) instead of per-attempt connectFailed avoids over-invalidating multi-A-record hosts where one IP is dead and OkHttp recovers via the next. - Media path: invalidation folded into MediaCallEventListener.finish alongside its existing timing logging. - Relay path: new DnsInvalidatingEventListener wired into OkHttpClientFactoryForRelays via eventListenerFactory. --- .../amethyst/service/okhttp/AmethystDns.kt | 23 ++++++--- .../okhttp/DnsInvalidatingEventListener.kt | 50 +++++++++++++++++++ .../service/okhttp/MediaCallEventListener.kt | 11 +++- .../okhttp/OkHttpClientFactoryForRelays.kt | 1 + .../service/okhttp/AmethystDnsTest.kt | 39 +++++++++++++++ 5 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index e67d4c638b..fa85d0c37f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.okhttp import okhttp3.Dns import java.net.InetAddress import java.net.UnknownHostException +import java.util.Locale import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ExecutionException @@ -85,25 +86,29 @@ class AmethystDns( private val dirty = AtomicBoolean(false) override fun lookup(hostname: String): List { - cache[hostname]?.let { entry -> + // DNS hostnames are case-insensitive; canonicalize so "Example.com" and "example.com" + // share a cache entry. OkHttp normally hands us lowercase, but custom callers may not. + val key = hostname.lowercase(Locale.ROOT) + + cache[key]?.let { entry -> val now = System.currentTimeMillis() if (entry.expiresAtMillis > now) { - return entry.unwrap(hostname) + return entry.unwrap(key) } // Soft-expired positive entry: serve stale, refresh in background. Negative // entries fall through to the sync path so transient failures recover quickly. if (entry.addresses.isNotEmpty()) { - triggerBackgroundRefresh(hostname) + triggerBackgroundRefresh(key) return entry.addresses } } val newFuture = CompletableFuture>() - val existing = inflight.putIfAbsent(hostname, newFuture) + val existing = inflight.putIfAbsent(key, newFuture) return if (existing == null) { - resolveAsLeader(hostname, newFuture) + resolveAsLeader(key, newFuture) } else { - awaitFollower(hostname, existing) + awaitFollower(key, existing) } } @@ -212,7 +217,8 @@ class AmethystDns( /** Drop a single host's cached entry. Call when a connection to it fails. */ fun invalidate(hostname: String) { - if (cache.remove(hostname) != null) dirty.set(true) + val key = hostname.lowercase(Locale.ROOT) + if (cache.remove(key) != null) dirty.set(true) } /** @@ -249,7 +255,8 @@ class AmethystDns( runCatching { InetAddress.getByName(literal) }.getOrNull() } if (addresses.isNotEmpty()) { - cache.putIfAbsent(record.hostname, Entry(addresses, record.expiresAtMillis)) + val key = record.hostname.lowercase(Locale.ROOT) + cache.putIfAbsent(key, Entry(addresses, record.expiresAtMillis)) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt new file mode 100644 index 0000000000..9feb9884ef --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.okhttp + +import okhttp3.Call +import okhttp3.EventListener +import java.io.IOException + +/** + * Drops a host's [AmethystDns] entry whenever an OkHttp call to it fails outright. We hook + * `callFailed` (final-stage signal after OkHttp has tried every address from the DNS lookup) + * rather than per-attempt `connectFailed`: a multi-A-record host with one bad IP fires the + * latter while OkHttp recovers via the next address, and we don't want to invalidate the cache + * just because the first IP was unreachable. + * + * Used by the relay client. The media path uses [MediaCallEventListener], which folds the same + * invalidation into its `finish` method alongside its existing timing logging. + */ +class DnsInvalidatingEventListener private constructor() : EventListener() { + override fun callFailed( + call: Call, + ioe: IOException, + ) { + AmethystDns.shared.invalidate(call.request().url.host) + } + + object Factory : EventListener.Factory { + private val INSTANCE = DnsInvalidatingEventListener() + + override fun create(call: Call): EventListener = INSTANCE + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt index 18875d661a..4397112a89 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt @@ -122,6 +122,16 @@ class MediaCallEventListener( call: Call, error: IOException?, ) { + val host = call.request().url.host + + // The whole call failed (after OkHttp exhausted every address from the DNS lookup). + // Drop the cached entry so the next attempt re-resolves instead of trying the same + // dead IPs for up to 24h. Per-attempt connectFailed isn't enough — a multi-A-record + // host can have one bad IP and OkHttp will recover by trying the next one. + if (error != null) { + AmethystDns.shared.invalidate(host) + } + val totalMs = (System.nanoTime() - callStartNanos) / 1_000_000 val isSlow = totalMs >= SLOW_CALL_THRESHOLD_MS val wasQueued = queuedAtStart > 0 @@ -129,7 +139,6 @@ class MediaCallEventListener( if (error == null && !isSlow && !wasQueued && !isDebug) return val ttfbMs = if (responseHeadersNanos > 0) (responseHeadersNanos - callStartNanos) / 1_000_000 else -1L - val host = call.request().url.host val reuseTag = if (connectionReused) "reused" else "new" val msg = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index fb63b34165..b5dc62b039 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -56,6 +56,7 @@ class OkHttpClientFactoryForRelays( .Builder() .dispatcher(myDispatcher) .dns(AmethystDns.shared) + .eventListenerFactory(DnsInvalidatingEventListener.Factory) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt index fcdcbfb733..45a86b6a63 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -392,6 +392,45 @@ class AmethystDnsTest { assertEquals("In-memory entry wins", listOf(ip("1.1.1.1")), dns.lookup("relay.example")) } + @Test + fun `lookup is case-insensitive`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("Example.COM") + dns.lookup("example.com") + dns.lookup("ExAmPlE.cOm") + + assertEquals("Mixed-case lookups should share one cache entry", 1, upstream.calls("example.com")) + } + + @Test + fun `invalidate is case-insensitive`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("example.com") + dns.invalidate("EXAMPLE.com") + dns.lookup("example.com") + + assertEquals(2, upstream.calls("example.com")) + } + + @Test + fun `restore lowercases hostnames so subsequent lookups hit`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("9.9.9.9")))) + val dns = AmethystDns(delegate = upstream) + + dns.restore( + listOf( + DnsCacheRecord("Example.COM", listOf("9.9.9.9"), System.currentTimeMillis() + 60_000), + ), + ) + + assertEquals(listOf(ip("9.9.9.9")), dns.lookup("example.com")) + assertEquals("Lookup should hit restored entry without upstream", 0, upstream.calls("example.com")) + } + @Test fun `dirty flag tracks positive writes`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) From eeb5f007329d3bffcb4a8e8ab7fc45a63a88864a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 20:44:12 +0000 Subject: [PATCH 07/11] refactor(okhttp): tidy AmethystDns internals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanical cleanups from a final review pass. No behavior change. - Drop the dead `private val xxxMillis = xxxMs` aliasing — just use the constructor params directly. - Extract positiveExpiry()/negativeExpiry() and evictIfOverCap() so putPositive and putNegative read as one line each. - Extract lookupAndCache() from resolveAsLeader so the leader's bookkeeping (re-check, complete future, remove inflight) is separate from the upstream/cache write logic. - Tighten awaitFollower's cause-handling. - Use runCatching in the refresh executor task. - Drop DnsCacheRecord's no-arg constructor — jacksonObjectMapper() registers the Kotlin module, which uses the primary constructor reflectively. - Rename `hostname` -> `host` in private methods that receive the already-normalized lowercase key. --- .../amethyst/service/okhttp/AmethystDns.kt | 121 ++++++++---------- 1 file changed, 53 insertions(+), 68 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index fa85d0c37f..b42a45061f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -72,15 +72,11 @@ import java.util.concurrent.atomic.AtomicBoolean class AmethystDns( private val delegate: Dns = Dns.SYSTEM, private val maxEntries: Int = 2000, - positiveTtlMs: Long = TimeUnit.HOURS.toMillis(24), - positiveTtlJitterMs: Long = TimeUnit.HOURS.toMillis(24), - negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), + private val positiveTtlMs: Long = TimeUnit.HOURS.toMillis(24), + private val positiveTtlJitterMs: Long = TimeUnit.HOURS.toMillis(24), + private val negativeTtlMs: Long = TimeUnit.SECONDS.toMillis(10), private val refreshExecutor: Executor = DEFAULT_REFRESH_EXECUTOR, ) : Dns { - private val positiveTtlMillis = positiveTtlMs - private val positiveTtlJitterMillis = positiveTtlJitterMs - private val negativeTtlMillis = negativeTtlMs - private val cache = ConcurrentHashMap() private val inflight = ConcurrentHashMap>>() private val dirty = AtomicBoolean(false) @@ -91,8 +87,7 @@ class AmethystDns( val key = hostname.lowercase(Locale.ROOT) cache[key]?.let { entry -> - val now = System.currentTimeMillis() - if (entry.expiresAtMillis > now) { + if (entry.expiresAtMillis > System.currentTimeMillis()) { return entry.unwrap(key) } // Soft-expired positive entry: serve stale, refresh in background. Negative @@ -105,103 +100,94 @@ class AmethystDns( val newFuture = CompletableFuture>() val existing = inflight.putIfAbsent(key, newFuture) - return if (existing == null) { - resolveAsLeader(key, newFuture) - } else { - awaitFollower(key, existing) - } + return if (existing == null) resolveAsLeader(key, newFuture) else awaitFollower(key, existing) } - private fun triggerBackgroundRefresh(hostname: String) { + private fun triggerBackgroundRefresh(host: String) { val refreshFuture = CompletableFuture>() // Coalesce: if a sync lookup or a prior refresh is already in flight, skip. - if (inflight.putIfAbsent(hostname, refreshFuture) != null) return + if (inflight.putIfAbsent(host, refreshFuture) != null) return try { + // The caller already got the stale answer; refresh failures are recorded on the + // future and (for UnknownHostException) demoted in the cache by lookupAndCache. refreshExecutor.execute { - try { - resolveAsLeader(hostname, refreshFuture) - } catch (_: Throwable) { - // Caller already got the stale answer; the failure is recorded on the - // future and (for UnknownHostException) demoted in the cache. - } + runCatching { resolveAsLeader(host, refreshFuture) } } } catch (_: RejectedExecutionException) { - inflight.remove(hostname, refreshFuture) + inflight.remove(host, refreshFuture) } } private fun resolveAsLeader( - hostname: String, + host: String, future: CompletableFuture>, ): List { try { // Re-check after claiming leadership: a peer may have refreshed the cache between // our miss and our putIfAbsent. Skips a duplicate getaddrinfo in that race. - val cached = cache[hostname] - val addresses = - if (cached != null && cached.expiresAtMillis > System.currentTimeMillis()) { - cached.unwrap(hostname) - } else { - try { - delegate.lookup(hostname).also { putPositive(hostname, it) } - } catch (e: UnknownHostException) { - putNegative(hostname) - throw e - } - } + val fresh = cache[host]?.takeIf { it.expiresAtMillis > System.currentTimeMillis() } + val addresses = fresh?.unwrap(host) ?: lookupAndCache(host) future.complete(addresses) return addresses } catch (e: Throwable) { future.completeExceptionally(e) throw e } finally { - inflight.remove(hostname, future) + inflight.remove(host, future) } } + private fun lookupAndCache(host: String): List = + try { + delegate.lookup(host).also { putPositive(host, it) } + } catch (e: UnknownHostException) { + putNegative(host) + throw e + } + private fun awaitFollower( - hostname: String, + host: String, future: CompletableFuture>, - ): List { + ): List = try { - val addresses = future.get() - return addresses.ifEmpty { throw UnknownHostException(hostname) } + future.get().ifEmpty { throw UnknownHostException(host) } } catch (e: ExecutionException) { - when (val cause = e.cause) { - is UnknownHostException -> throw cause - null -> throw UnknownHostException(hostname) - else -> throw UnknownHostException(hostname).apply { initCause(cause) } - } + val cause = e.cause + if (cause is UnknownHostException) throw cause + throw UnknownHostException(host).apply { if (cause != null) initCause(cause) } } catch (e: InterruptedException) { Thread.currentThread().interrupt() - throw UnknownHostException(hostname).apply { initCause(e) } + throw UnknownHostException(host).apply { initCause(e) } } - } private fun putPositive( - hostname: String, + host: String, addresses: List, ) { - // Jitter the expiry so a burst of co-written entries (e.g. ~700 relay reconnects at app - // start) doesn't all expire at the same instant 24h later. - val jitter = - if (positiveTtlJitterMillis > 0L) { - ThreadLocalRandom.current().nextLong(positiveTtlJitterMillis) - } else { - 0L - } - cache[hostname] = Entry(addresses, System.currentTimeMillis() + positiveTtlMillis + jitter) + cache[host] = Entry(addresses, positiveExpiry()) dirty.set(true) - if (cache.size > maxEntries) evictExpired() + evictIfOverCap() } - private fun putNegative(hostname: String) { - cache[hostname] = Entry(emptyList(), System.currentTimeMillis() + negativeTtlMillis) + private fun putNegative(host: String) { // Negative entries are never persisted, so they don't dirty the cache. - if (cache.size > maxEntries) evictExpired() + cache[host] = Entry(emptyList(), negativeExpiry()) + evictIfOverCap() } - private fun evictExpired() { + /** + * Jittered expiry so a burst of co-written entries (e.g. ~700 relay reconnects at app start) + * doesn't all expire at the same instant 24h later. + */ + private fun positiveExpiry(): Long { + val jitter = if (positiveTtlJitterMs > 0L) ThreadLocalRandom.current().nextLong(positiveTtlJitterMs) else 0L + return System.currentTimeMillis() + positiveTtlMs + jitter + } + + private fun negativeExpiry(): Long = System.currentTimeMillis() + negativeTtlMs + + private fun evictIfOverCap() { + if (cache.size <= maxEntries) return val now = System.currentTimeMillis() val it = cache.entries.iterator() while (it.hasNext()) { @@ -232,6 +218,8 @@ class AmethystDns( for ((host, entry) in cache) { if (entry.addresses.isEmpty()) continue if (entry.expiresAtMillis <= now) continue + // hostAddress is platform-typed (String!); mapNotNull narrows to String and stays + // defensive against the rare case where it might be null. val ips = entry.addresses.mapNotNull { it.hostAddress } if (ips.isNotEmpty()) { out += DnsCacheRecord(host, ips, entry.expiresAtMillis) @@ -273,7 +261,7 @@ class AmethystDns( val addresses: List, val expiresAtMillis: Long, ) { - fun unwrap(hostname: String): List = addresses.ifEmpty { throw UnknownHostException(hostname) } + fun unwrap(host: String): List = addresses.ifEmpty { throw UnknownHostException(host) } } companion object { @@ -299,7 +287,4 @@ data class DnsCacheRecord( val hostname: String, val addresses: List, val expiresAtMillis: Long, -) { - // No-arg constructor for Jackson. - constructor() : this("", emptyList(), 0L) -} +) From 6240e771f86ea676368fda1521914b5e85bf728f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 20:48:12 +0000 Subject: [PATCH 08/11] refactor(okhttp): inject AmethystDns instead of using a singleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the AmethystDns.shared lazy companion. Construct one amethystDns in AppModules and thread it through: - DualHttpClientManager / OkHttpClientFactory - DualHttpClientManagerForRelays / OkHttpClientFactoryForRelays - MediaCallEventListenerFactory / MediaCallEventListener - DnsInvalidatingEventListener.Factory - AmethystDnsStore This matches the dependency-injection style the rest of AppModules uses and lets tests inject a mock Dns where needed. Behavior is unchanged — every consumer still shares the same single instance, just by construction rather than by a static singleton. --- .../com/vitorpamplona/amethyst/AppModules.kt | 16 ++++++++++++---- .../amethyst/service/okhttp/AmethystDns.kt | 6 ------ .../amethyst/service/okhttp/AmethystDnsStore.kt | 2 +- .../okhttp/DnsInvalidatingEventListener.kt | 15 ++++++++++----- .../service/okhttp/DualHttpClientManager.kt | 3 ++- .../okhttp/DualHttpClientManagerForRelays.kt | 3 ++- .../service/okhttp/MediaCallEventListener.kt | 6 ++++-- .../service/okhttp/OkHttpClientFactory.kt | 5 +++-- .../okhttp/OkHttpClientFactoryForRelays.kt | 5 +++-- 9 files changed, 37 insertions(+), 24 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index af11cf8115..7555e6c5b4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -53,6 +53,7 @@ import com.vitorpamplona.amethyst.service.location.LocationState import com.vitorpamplona.amethyst.service.notifications.AlwaysOnNotificationServiceManager import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher import com.vitorpamplona.amethyst.service.notifications.PokeyReceiver +import com.vitorpamplona.amethyst.service.okhttp.AmethystDns import com.vitorpamplona.amethyst.service.okhttp.AmethystDnsStore import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManager import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManagerForRelays @@ -202,10 +203,15 @@ class AppModules( // Key cache service to download and decrypt encrypted files before caching them. val keyCache = EncryptionKeyCache() - // Persists the shared DNS resolver's positive cache across process restarts so cold starts - // don't pay ~700 sync getaddrinfo calls. Restored entries fall through to the - // stale-while-revalidate path on first lookup. - val dnsStore = AmethystDnsStore(appContext) + // Concurrent, caching DNS resolver shared by every OkHttp client built below — a host + // resolved for an image fetch is reused when a relay handshake or NIP-05 lookup hits the + // same host. + val amethystDns = AmethystDns() + + // Persists [amethystDns]'s positive cache across process restarts so cold starts don't pay + // ~700 sync getaddrinfo calls. Restored entries fall through to the stale-while-revalidate + // path on first lookup. + val dnsStore = AmethystDnsStore(appContext, amethystDns) // manages all the other connections separately from relays. val okHttpClients = @@ -215,6 +221,7 @@ class AppModules( isMobileDataProvider = connManager.isMobileOrNull, keyCache = keyCache, scope = applicationIOScope, + dns = amethystDns, ) // Offers easy methods to know when connections are happening through Tor or not @@ -296,6 +303,7 @@ class AppModules( proxyPortProvider = torManager.activePortOrNull, isMobileDataProvider = connManager.isMobileOrNull, scope = applicationIOScope, + dns = amethystDns, ) // Connects the INostrClient class with okHttp diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index b42a45061f..754a33a2ac 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -273,12 +273,6 @@ class AmethystDns( Executors.newFixedThreadPool(8) { r -> Thread(r, "amethyst-dns-refresh").apply { isDaemon = true } } - - /** - * Process-wide instance shared by every OkHttp client built in the app, so a host resolved - * for an image fetch is reused when a relay handshake or NIP-05 lookup hits the same host. - */ - val shared: AmethystDns by lazy { AmethystDns() } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt index b9424b2ed3..0ae538d52d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt @@ -40,7 +40,7 @@ import com.vitorpamplona.quartz.utils.Log */ class AmethystDnsStore( private val context: Context, - private val dns: AmethystDns = AmethystDns.shared, + private val dns: AmethystDns, ) { private val prefs by lazy { context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt index 9feb9884ef..3da60a7aca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt @@ -34,17 +34,22 @@ import java.io.IOException * Used by the relay client. The media path uses [MediaCallEventListener], which folds the same * invalidation into its `finish` method alongside its existing timing logging. */ -class DnsInvalidatingEventListener private constructor() : EventListener() { +class DnsInvalidatingEventListener( + private val dns: AmethystDns, +) : EventListener() { override fun callFailed( call: Call, ioe: IOException, ) { - AmethystDns.shared.invalidate(call.request().url.host) + dns.invalidate(call.request().url.host) } - object Factory : EventListener.Factory { - private val INSTANCE = DnsInvalidatingEventListener() + /** Per-client factory. The listener is stateless, so the same instance serves every call. */ + class Factory( + dns: AmethystDns, + ) : EventListener.Factory { + private val listener = DnsInvalidatingEventListener(dns) - override fun create(call: Call): EventListener = INSTANCE + override fun create(call: Call): EventListener = listener } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt index b0c4d0ed22..460f90250a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt @@ -38,8 +38,9 @@ class DualHttpClientManager( isMobileDataProvider: StateFlow, keyCache: EncryptionKeyCache, scope: CoroutineScope, + dns: AmethystDns, ) : IHttpClientManager { - val factory = OkHttpClientFactory(keyCache, userAgent) + val factory = OkHttpClientFactory(keyCache, userAgent, dns) val defaultHttpClient: StateFlow = combine(proxyPortProvider, isMobileDataProvider) { proxy, mobile -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt index 76413155f7..891ccfae9c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt @@ -35,8 +35,9 @@ class DualHttpClientManagerForRelays( proxyPortProvider: StateFlow, isMobileDataProvider: StateFlow, scope: CoroutineScope, + dns: AmethystDns, ) : IHttpClientManager { - val factory = OkHttpClientFactoryForRelays(userAgent) + val factory = OkHttpClientFactoryForRelays(userAgent, dns) val defaultHttpClient: StateFlow = combine(proxyPortProvider, isMobileDataProvider) { proxy, mobile -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt index 4397112a89..b7b399d42b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt @@ -44,6 +44,7 @@ import java.net.Proxy class MediaCallEventListener( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, + private val dns: AmethystDns, ) : EventListener() { private var callStartNanos = 0L private var dnsStartNanos = 0L @@ -129,7 +130,7 @@ class MediaCallEventListener( // dead IPs for up to 24h. Per-attempt connectFailed isn't enough — a multi-A-record // host can have one bad IP and OkHttp will recover by trying the next one. if (error != null) { - AmethystDns.shared.invalidate(host) + dns.invalidate(host) } val totalMs = (System.nanoTime() - callStartNanos) / 1_000_000 @@ -178,6 +179,7 @@ class MediaCallEventListener( class MediaCallEventListenerFactory( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, + private val dns: AmethystDns, ) : EventListener.Factory { - override fun create(call: Call): EventListener = MediaCallEventListener(dispatcher, connectionPool) + override fun create(call: Call): EventListener = MediaCallEventListener(dispatcher, connectionPool, dns) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt index 6150ad33ff..de24b6a864 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt @@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit class OkHttpClientFactory( keyCache: EncryptionKeyCache, val userAgent: String, + private val dns: AmethystDns, ) { // val logging = LoggingInterceptor() val keyDecryptor = EncryptedBlobInterceptor(keyCache) @@ -63,8 +64,8 @@ class OkHttpClientFactory( .Builder() .dispatcher(dispatcher) .connectionPool(connectionPool) - .dns(AmethystDns.shared) - .eventListenerFactory(MediaCallEventListenerFactory(dispatcher, connectionPool)) + .dns(dns) + .eventListenerFactory(MediaCallEventListenerFactory(dispatcher, connectionPool, dns)) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index b5dc62b039..10983a6d4d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -29,6 +29,7 @@ import java.time.Duration class OkHttpClientFactoryForRelays( userAgent: String, + private val dns: AmethystDns, ) { companion object { // by picking a random proxy port, the connection will fail as it should. @@ -55,8 +56,8 @@ class OkHttpClientFactoryForRelays( OkHttpClient .Builder() .dispatcher(myDispatcher) - .dns(AmethystDns.shared) - .eventListenerFactory(DnsInvalidatingEventListener.Factory) + .dns(dns) + .eventListenerFactory(DnsInvalidatingEventListener.Factory(dns)) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) From e86871a8687cca81148c12a0e2be7a316776e2ee Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 21:01:03 +0000 Subject: [PATCH 09/11] fix(okhttp): close save-race window and reject empty positive results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness fixes from the audit: 1. Save race. The previous order (snapshot -> write -> clearDirty) could lose a putPositive that landed between the write and the clearDirty: clearDirty unconditionally wiped the flag, so the just-written entry would never be persisted. Replace clearDirty with tryClearDirty (compareAndSet), called BEFORE the snapshot — any concurrent put then re-marks dirty and is captured by the next save. Add markDirty so the store can re-flag on write failure. Also drop the buggy clearDirty in load(): if puts happened between AmethystDns construction and load completion, we used to silently discard their dirty signal. restore() never marks dirty, so the manual clear was both unnecessary and incorrect. 2. Empty positive results. A misbehaving Dns delegate returning emptyList() would land in the cache as a positive entry — harmless in lookup semantics (unwrap throws on empty) but wrongly persisted to disk and dirty-flagged. Treat empty as UnknownHostException so it goes through putNegative. Add three tests: - failed refresh demotes a stale positive entry to negative - failed lookup does not mark cache dirty - refresh executor rejection cleans up the inflight slot --- .../amethyst/service/okhttp/AmethystDns.kt | 20 +++-- .../service/okhttp/AmethystDnsStore.kt | 16 ++-- .../service/okhttp/AmethystDnsTest.kt | 81 ++++++++++++++++++- 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index 754a33a2ac..3d1d7a725c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -139,7 +139,9 @@ class AmethystDns( private fun lookupAndCache(host: String): List = try { - delegate.lookup(host).also { putPositive(host, it) } + val addresses = delegate.lookup(host).ifEmpty { throw UnknownHostException(host) } + putPositive(host, addresses) + addresses } catch (e: UnknownHostException) { putNegative(host) throw e @@ -249,12 +251,20 @@ class AmethystDns( } } - /** True if the cache has changed since the last [clearDirty]. */ + /** True if the cache has changed since the last [tryClearDirty] / [markDirty] / construction. */ fun isDirty(): Boolean = dirty.get() - /** Marks the cache clean. Call after a successful persist. */ - fun clearDirty() { - dirty.set(false) + /** + * Atomically clears the dirty flag if it was set. Returns `true` if the caller observed the + * flag in the dirty state (and is now responsible for persisting). Use this to bracket a save + * — clearing **before** taking a snapshot ensures any concurrent [putPositive] re-marks dirty + * and is captured by the next save instead of being silently lost. + */ + fun tryClearDirty(): Boolean = dirty.compareAndSet(true, false) + + /** Re-marks the cache dirty. For a store to call when its persist attempt fails. */ + fun markDirty() { + dirty.set(true) } private class Entry( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt index 0ae538d52d..f64335c441 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt @@ -59,10 +59,10 @@ class AmethystDnsStore( prefs.edit().remove(KEY_CACHE).apply() return } + // restore() uses putIfAbsent and never marks dirty, so we deliberately do NOT clear the + // dirty flag here — any concurrent put that happened before load completed must still be + // persisted on the next save. dns.restore(records) - // Restoring entries that already existed in memory is a no-op, but the act of loading - // shouldn't mark the cache dirty. - dns.clearDirty() Log.d(TAG) { "Restored ${records.size} DNS cache entries" } } @@ -71,15 +71,19 @@ class AmethystDnsStore( * I/O — call from a background thread. */ fun save() { - if (!dns.isDirty()) return - val records = dns.snapshot() + // Clear BEFORE snapshot so any put racing with the snapshot/write re-marks dirty and is + // captured by the next save instead of being silently lost. compareAndSet ensures two + // concurrent saves don't both proceed. + if (!dns.tryClearDirty()) return try { + val records = dns.snapshot() val json = MAPPER.writeValueAsString(records) prefs.edit().putString(KEY_CACHE, json).apply() - dns.clearDirty() Log.d(TAG) { "Persisted ${records.size} DNS cache entries" } } catch (t: Throwable) { Log.w(TAG) { "Failed to persist DNS cache: ${t.message}" } + // Restore the dirty signal so the next save retries. + dns.markDirty() } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt index 45a86b6a63..189a9ecb9f 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -32,6 +32,7 @@ import java.net.UnknownHostException import java.util.concurrent.CountDownLatch import java.util.concurrent.Executor import java.util.concurrent.Executors +import java.util.concurrent.RejectedExecutionException import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference @@ -445,8 +446,86 @@ class AmethystDnsTest { dns.lookup("a.example") assertTrue("First positive write dirties cache", dns.isDirty()) - dns.clearDirty() + assertTrue("tryClearDirty reports prior dirty state", dns.tryClearDirty()) dns.lookup("a.example") // cache hit, no write assertFalse("Cache hit does not dirty", dns.isDirty()) + assertFalse("tryClearDirty on already-clean returns false", dns.tryClearDirty()) + } + + @Test + fun `failed lookup does not mark cache dirty`() { + val upstream = CountingDns(emptyMap()) + val dns = AmethystDns(delegate = upstream) + + assertFalse(dns.isDirty()) + runCatching { dns.lookup("missing.example") } + assertFalse("Negative entry must not dirty the cache (it isn't persisted)", dns.isDirty()) + } + + @Test + fun `failed refresh demotes stale positive entry to negative`() { + val responses = AtomicReference?>(listOf(ip("1.2.3.4"))) + val upstream = + Dns { hostname -> + responses.get() ?: throw UnknownHostException(hostname) + } + val syncRefresh = Executor { it.run() } + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 1, + positiveTtlJitterMs = 0, + negativeTtlMs = 60_000, + refreshExecutor = syncRefresh, + ) + + // Populate, then let it go stale. + dns.lookup("a.example") + Thread.sleep(20) + + // Make upstream fail. + responses.set(null) + + // Stale read returns the cached value AND triggers a refresh that fails. + assertEquals(listOf(ip("1.2.3.4")), dns.lookup("a.example")) + + // Entry should now be negative — next caller gets a fresh failure rather than + // forever-stale wrong IPs. + assertThrows(UnknownHostException::class.java) { dns.lookup("a.example") } + } + + @Test + fun `refresh executor rejection cleans up the inflight slot`() { + val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) + val rejecting = Executor { throw RejectedExecutionException("test") } + val dns = + AmethystDns( + delegate = upstream, + positiveTtlMs = 1, + positiveTtlJitterMs = 0, + refreshExecutor = rejecting, + ) + + dns.lookup("a.example") + Thread.sleep(20) + + // Stale read tries to schedule a refresh; the executor rejects it. The caller still + // gets the stale answer. + assertEquals(listOf(ip("1.2.3.4")), dns.lookup("a.example")) + + // If the rejected future was leaked into inflight, a subsequent cache-miss lookup for + // the same host would block forever in awaitFollower. Force a cache miss via invalidate + // and run the lookup with a hard timeout to verify the slot was freed. + dns.invalidate("a.example") + val pool = Executors.newSingleThreadExecutor() + try { + val result = + pool + .submit> { dns.lookup("a.example") } + .get(2, TimeUnit.SECONDS) + assertEquals(listOf(ip("1.2.3.4")), result) + } finally { + pool.shutdownNow() + } } } From e173daf3c9e31f0afbbc9f1d51bd32bab7ce37c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 21:14:12 +0000 Subject: [PATCH 10/11] refactor(okhttp): clearer names in DNS resolver - evictIfOverCap -> purgeExpiredIfOverCap. The method only purges expired entries; "evict" implied LRU behavior. - triggerBackgroundRefresh -> scheduleBackgroundRefresh. Better reflects that we hand the work to an executor that may queue it. - fresh -> freshEntry in resolveAsLeader. The bare adjective read as a boolean. - KEY_CACHE -> KEY_SNAPSHOT in AmethystDnsStore. The constant identifier now matches what we actually store via dns.snapshot(); the string value is unchanged. --- .../amethyst/service/okhttp/AmethystDns.kt | 14 +++++++------- .../amethyst/service/okhttp/AmethystDnsStore.kt | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index 3d1d7a725c..98101a8f7d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -93,7 +93,7 @@ class AmethystDns( // Soft-expired positive entry: serve stale, refresh in background. Negative // entries fall through to the sync path so transient failures recover quickly. if (entry.addresses.isNotEmpty()) { - triggerBackgroundRefresh(key) + scheduleBackgroundRefresh(key) return entry.addresses } } @@ -103,7 +103,7 @@ class AmethystDns( return if (existing == null) resolveAsLeader(key, newFuture) else awaitFollower(key, existing) } - private fun triggerBackgroundRefresh(host: String) { + private fun scheduleBackgroundRefresh(host: String) { val refreshFuture = CompletableFuture>() // Coalesce: if a sync lookup or a prior refresh is already in flight, skip. if (inflight.putIfAbsent(host, refreshFuture) != null) return @@ -125,8 +125,8 @@ class AmethystDns( try { // Re-check after claiming leadership: a peer may have refreshed the cache between // our miss and our putIfAbsent. Skips a duplicate getaddrinfo in that race. - val fresh = cache[host]?.takeIf { it.expiresAtMillis > System.currentTimeMillis() } - val addresses = fresh?.unwrap(host) ?: lookupAndCache(host) + val freshEntry = cache[host]?.takeIf { it.expiresAtMillis > System.currentTimeMillis() } + val addresses = freshEntry?.unwrap(host) ?: lookupAndCache(host) future.complete(addresses) return addresses } catch (e: Throwable) { @@ -168,13 +168,13 @@ class AmethystDns( ) { cache[host] = Entry(addresses, positiveExpiry()) dirty.set(true) - evictIfOverCap() + purgeExpiredIfOverCap() } private fun putNegative(host: String) { // Negative entries are never persisted, so they don't dirty the cache. cache[host] = Entry(emptyList(), negativeExpiry()) - evictIfOverCap() + purgeExpiredIfOverCap() } /** @@ -188,7 +188,7 @@ class AmethystDns( private fun negativeExpiry(): Long = System.currentTimeMillis() + negativeTtlMs - private fun evictIfOverCap() { + private fun purgeExpiredIfOverCap() { if (cache.size <= maxEntries) return val now = System.currentTimeMillis() val it = cache.entries.iterator() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt index f64335c441..d80f34dd28 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt @@ -50,13 +50,13 @@ class AmethystDnsStore( * call from a background thread. */ fun load() { - val json = prefs.getString(KEY_CACHE, null) ?: return + val json = prefs.getString(KEY_SNAPSHOT, null) ?: return val records = try { MAPPER.readValue>(json) } catch (t: Throwable) { Log.w(TAG) { "Dropping corrupt DNS cache blob: ${t.message}" } - prefs.edit().remove(KEY_CACHE).apply() + prefs.edit().remove(KEY_SNAPSHOT).apply() return } // restore() uses putIfAbsent and never marks dirty, so we deliberately do NOT clear the @@ -78,7 +78,7 @@ class AmethystDnsStore( try { val records = dns.snapshot() val json = MAPPER.writeValueAsString(records) - prefs.edit().putString(KEY_CACHE, json).apply() + prefs.edit().putString(KEY_SNAPSHOT, json).apply() Log.d(TAG) { "Persisted ${records.size} DNS cache entries" } } catch (t: Throwable) { Log.w(TAG) { "Failed to persist DNS cache: ${t.message}" } @@ -89,13 +89,13 @@ class AmethystDnsStore( /** Force-clear the on-disk cache. Useful for diagnostics or when the user wipes data. */ fun clear() { - prefs.edit().remove(KEY_CACHE).apply() + prefs.edit().remove(KEY_SNAPSHOT).apply() } companion object { private const val TAG = "AmethystDnsStore" private const val PREFS_NAME = "amethyst_dns_cache" - private const val KEY_CACHE = "dns_cache_v1" + private const val KEY_SNAPSHOT = "dns_cache_v1" private val MAPPER = jacksonObjectMapper() } } From b9a260f3ceaba8013f6857e448c016b177bb6b16 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 21:26:46 +0000 Subject: [PATCH 11/11] refactor(okhttp): rename AmethystDns -> SurgeDns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "AmethystDns" identified the owner; "SurgeDns" identifies the behavior. The class is built to absorb sudden bursts of concurrent DNS work — 700 relays reconnecting, a feed scrolling through dozens of media hosts, a profile screen requesting a NIP-05 — without falling over: lock-free reads, single-flight coalescing, and stale-while-revalidate so recurring hosts never block. File renames: - AmethystDns.kt -> SurgeDns.kt - AmethystDnsStore.kt -> SurgeDnsStore.kt - AmethystDnsTest.kt -> SurgeDnsTest.kt Symbol renames in every touchpoint (AppModules, factories, managers, event listeners): AmethystDns -> SurgeDns, amethystDns -> surgeDns. --- .../com/vitorpamplona/amethyst/AppModules.kt | 14 +++--- .../okhttp/DnsInvalidatingEventListener.kt | 6 +-- .../service/okhttp/DualHttpClientManager.kt | 2 +- .../okhttp/DualHttpClientManagerForRelays.kt | 2 +- .../service/okhttp/MediaCallEventListener.kt | 4 +- .../service/okhttp/OkHttpClientFactory.kt | 2 +- .../okhttp/OkHttpClientFactoryForRelays.kt | 2 +- .../okhttp/{AmethystDns.kt => SurgeDns.kt} | 4 +- .../{AmethystDnsStore.kt => SurgeDnsStore.kt} | 10 ++--- .../{AmethystDnsTest.kt => SurgeDnsTest.kt} | 44 +++++++++---------- 10 files changed, 45 insertions(+), 45 deletions(-) rename amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/{AmethystDns.kt => SurgeDns.kt} (99%) rename amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/{AmethystDnsStore.kt => SurgeDnsStore.kt} (93%) rename amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/{AmethystDnsTest.kt => SurgeDnsTest.kt} (95%) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 7555e6c5b4..e0e0dc6d04 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -53,12 +53,12 @@ import com.vitorpamplona.amethyst.service.location.LocationState import com.vitorpamplona.amethyst.service.notifications.AlwaysOnNotificationServiceManager import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher import com.vitorpamplona.amethyst.service.notifications.PokeyReceiver -import com.vitorpamplona.amethyst.service.okhttp.AmethystDns -import com.vitorpamplona.amethyst.service.okhttp.AmethystDnsStore import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManager import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManagerForRelays import com.vitorpamplona.amethyst.service.okhttp.EncryptionKeyCache import com.vitorpamplona.amethyst.service.okhttp.OkHttpWebSocket +import com.vitorpamplona.amethyst.service.okhttp.SurgeDns +import com.vitorpamplona.amethyst.service.okhttp.SurgeDnsStore import com.vitorpamplona.amethyst.service.playback.diskCache.VideoCache import com.vitorpamplona.amethyst.service.playback.diskCache.VideoCacheFactory import com.vitorpamplona.amethyst.service.playback.pip.BackgroundMedia @@ -206,12 +206,12 @@ class AppModules( // Concurrent, caching DNS resolver shared by every OkHttp client built below — a host // resolved for an image fetch is reused when a relay handshake or NIP-05 lookup hits the // same host. - val amethystDns = AmethystDns() + val surgeDns = SurgeDns() - // Persists [amethystDns]'s positive cache across process restarts so cold starts don't pay + // Persists [surgeDns]'s positive cache across process restarts so cold starts don't pay // ~700 sync getaddrinfo calls. Restored entries fall through to the stale-while-revalidate // path on first lookup. - val dnsStore = AmethystDnsStore(appContext, amethystDns) + val dnsStore = SurgeDnsStore(appContext, surgeDns) // manages all the other connections separately from relays. val okHttpClients = @@ -221,7 +221,7 @@ class AppModules( isMobileDataProvider = connManager.isMobileOrNull, keyCache = keyCache, scope = applicationIOScope, - dns = amethystDns, + dns = surgeDns, ) // Offers easy methods to know when connections are happening through Tor or not @@ -303,7 +303,7 @@ class AppModules( proxyPortProvider = torManager.activePortOrNull, isMobileDataProvider = connManager.isMobileOrNull, scope = applicationIOScope, - dns = amethystDns, + dns = surgeDns, ) // Connects the INostrClient class with okHttp diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt index 3da60a7aca..5696529945 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt @@ -25,7 +25,7 @@ import okhttp3.EventListener import java.io.IOException /** - * Drops a host's [AmethystDns] entry whenever an OkHttp call to it fails outright. We hook + * Drops a host's [SurgeDns] entry whenever an OkHttp call to it fails outright. We hook * `callFailed` (final-stage signal after OkHttp has tried every address from the DNS lookup) * rather than per-attempt `connectFailed`: a multi-A-record host with one bad IP fires the * latter while OkHttp recovers via the next address, and we don't want to invalidate the cache @@ -35,7 +35,7 @@ import java.io.IOException * invalidation into its `finish` method alongside its existing timing logging. */ class DnsInvalidatingEventListener( - private val dns: AmethystDns, + private val dns: SurgeDns, ) : EventListener() { override fun callFailed( call: Call, @@ -46,7 +46,7 @@ class DnsInvalidatingEventListener( /** Per-client factory. The listener is stateless, so the same instance serves every call. */ class Factory( - dns: AmethystDns, + dns: SurgeDns, ) : EventListener.Factory { private val listener = DnsInvalidatingEventListener(dns) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt index 460f90250a..32679800f3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt @@ -38,7 +38,7 @@ class DualHttpClientManager( isMobileDataProvider: StateFlow, keyCache: EncryptionKeyCache, scope: CoroutineScope, - dns: AmethystDns, + dns: SurgeDns, ) : IHttpClientManager { val factory = OkHttpClientFactory(keyCache, userAgent, dns) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt index 891ccfae9c..d190c885d9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt @@ -35,7 +35,7 @@ class DualHttpClientManagerForRelays( proxyPortProvider: StateFlow, isMobileDataProvider: StateFlow, scope: CoroutineScope, - dns: AmethystDns, + dns: SurgeDns, ) : IHttpClientManager { val factory = OkHttpClientFactoryForRelays(userAgent, dns) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt index b7b399d42b..de863490dd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt @@ -44,7 +44,7 @@ import java.net.Proxy class MediaCallEventListener( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, - private val dns: AmethystDns, + private val dns: SurgeDns, ) : EventListener() { private var callStartNanos = 0L private var dnsStartNanos = 0L @@ -179,7 +179,7 @@ class MediaCallEventListener( class MediaCallEventListenerFactory( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, - private val dns: AmethystDns, + private val dns: SurgeDns, ) : EventListener.Factory { override fun create(call: Call): EventListener = MediaCallEventListener(dispatcher, connectionPool, dns) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt index de24b6a864..d208b511e0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt @@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit class OkHttpClientFactory( keyCache: EncryptionKeyCache, val userAgent: String, - private val dns: AmethystDns, + private val dns: SurgeDns, ) { // val logging = LoggingInterceptor() val keyDecryptor = EncryptedBlobInterceptor(keyCache) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index 10983a6d4d..a5e3619a60 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -29,7 +29,7 @@ import java.time.Duration class OkHttpClientFactoryForRelays( userAgent: String, - private val dns: AmethystDns, + private val dns: SurgeDns, ) { companion object { // by picking a random proxy port, the connection will fail as it should. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDns.kt similarity index 99% rename from amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt rename to amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDns.kt index 98101a8f7d..50bf1449f2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDns.kt @@ -69,7 +69,7 @@ import java.util.concurrent.atomic.AtomicBoolean * (unavoidable — there's nothing to serve stale yet), and followers waiting on that first * lookup block on `future.get()`. Background refreshes never block any caller. */ -class AmethystDns( +class SurgeDns( private val delegate: Dns = Dns.SYSTEM, private val maxEntries: Int = 2000, private val positiveTtlMs: Long = TimeUnit.HOURS.toMillis(24), @@ -286,7 +286,7 @@ class AmethystDns( } } -/** Persistable record. Public so [AmethystDnsStore] can serialize it via Jackson. */ +/** Persistable record. Public so [SurgeDnsStore] can serialize it via Jackson. */ data class DnsCacheRecord( val hostname: String, val addresses: List, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsStore.kt similarity index 93% rename from amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt rename to amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsStore.kt index d80f34dd28..c460f12107 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsStore.kt @@ -26,7 +26,7 @@ import com.fasterxml.jackson.module.kotlin.readValue import com.vitorpamplona.quartz.utils.Log /** - * Persists [AmethystDns]'s positive cache to a small `SharedPreferences` blob so the resolver + * Persists [SurgeDns]'s positive cache to a small `SharedPreferences` blob so the resolver * starts hot after a process restart. * * Cold starts are the resolver's worst case — every host is a sync `getaddrinfo`. With a @@ -38,15 +38,15 @@ import com.vitorpamplona.quartz.utils.Log * list, Coil's image cache, and the system resolver's own state. ~700 entries × ~80 bytes ≈ * ~55 KB of JSON. */ -class AmethystDnsStore( +class SurgeDnsStore( private val context: Context, - private val dns: AmethystDns, + private val dns: SurgeDns, ) { private val prefs by lazy { context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) } /** * Read the persisted snapshot and merge it into the resolver. Existing in-memory entries - * are preserved (see [AmethystDns.restore]). Safe to call once at app start. Blocking I/O — + * are preserved (see [SurgeDns.restore]). Safe to call once at app start. Blocking I/O — * call from a background thread. */ fun load() { @@ -93,7 +93,7 @@ class AmethystDnsStore( } companion object { - private const val TAG = "AmethystDnsStore" + private const val TAG = "SurgeDnsStore" private const val PREFS_NAME = "amethyst_dns_cache" private const val KEY_SNAPSHOT = "dns_cache_v1" private val MAPPER = jacksonObjectMapper() diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt similarity index 95% rename from amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt rename to amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt index 189a9ecb9f..ff7ad1fd68 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt @@ -37,7 +37,7 @@ import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference -class AmethystDnsTest { +class SurgeDnsTest { private fun ip(value: String) = InetAddress.getByName(value) private class CountingDns( @@ -71,7 +71,7 @@ class AmethystDnsTest { @Test fun `cache hit avoids second upstream call`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) val first = dns.lookup("a.example") val second = dns.lookup("a.example") @@ -84,7 +84,7 @@ class AmethystDnsTest { @Test fun `negative cache short-circuits subsequent lookups`() { val upstream = CountingDns(emptyMap()) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } assertThrows(UnknownHostException::class.java) { dns.lookup("missing.example") } @@ -97,7 +97,7 @@ class AmethystDnsTest { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) val syncRefresh = Executor { it.run() } val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 1, positiveTtlJitterMs = 0, @@ -117,7 +117,7 @@ class AmethystDnsTest { fun `expired negative entry does not stale-while-revalidate`() { val upstream = CountingDns(emptyMap()) val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlJitterMs = 0, negativeTtlMs = 1, @@ -142,7 +142,7 @@ class AmethystDnsTest { } val syncRefresh = Executor { it.run() } val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 1, positiveTtlJitterMs = 0, @@ -185,7 +185,7 @@ class AmethystDnsTest { } val pool = Executors.newFixedThreadPool(8) val dns = - AmethystDns( + SurgeDns( delegate = dynamic, positiveTtlMs = 1, positiveTtlJitterMs = 0, @@ -225,7 +225,7 @@ class AmethystDnsTest { @Test fun `concurrent lookups for the same host coalesce to one upstream call`() { val gated = GatedDns(mapOf("hot.example" to listOf(ip("9.9.9.9")))) - val dns = AmethystDns(delegate = gated) + val dns = SurgeDns(delegate = gated) val pool = Executors.newFixedThreadPool(8) try { @@ -263,7 +263,7 @@ class AmethystDnsTest { parallelism.decrementAndGet() } } - val dns = AmethystDns(delegate = instrumented) + val dns = SurgeDns(delegate = instrumented) val pool = Executors.newFixedThreadPool(2) try { @@ -289,7 +289,7 @@ class AmethystDnsTest { @Test fun `invalidate clears cache so next lookup hits upstream`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) dns.lookup("a.example") dns.invalidate() @@ -307,7 +307,7 @@ class AmethystDnsTest { "b.example" to listOf(ip("5.6.7.8")), ), ) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) dns.lookup("a.example") dns.lookup("b.example") @@ -329,7 +329,7 @@ class AmethystDnsTest { ), ) val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 60_000, positiveTtlJitterMs = 0, @@ -346,7 +346,7 @@ class AmethystDnsTest { @Test fun `restore replays cached entries without hitting upstream`() { val upstream = CountingDns(mapOf("relay.example" to listOf(ip("9.9.9.9")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) val expiresAt = System.currentTimeMillis() + 60_000 dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiresAt))) @@ -358,7 +358,7 @@ class AmethystDnsTest { @Test fun `restore drops entries already expired on disk`() { val upstream = CountingDns(mapOf("relay.example" to listOf(ip("9.9.9.9")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) val expiredAt = System.currentTimeMillis() - 1_000 dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiredAt))) @@ -371,7 +371,7 @@ class AmethystDnsTest { fun `restore does not overwrite a fresh in-memory entry`() { val upstream = CountingDns(mapOf("relay.example" to listOf(ip("1.1.1.1")))) val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 60_000, positiveTtlJitterMs = 0, @@ -396,7 +396,7 @@ class AmethystDnsTest { @Test fun `lookup is case-insensitive`() { val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) dns.lookup("Example.COM") dns.lookup("example.com") @@ -408,7 +408,7 @@ class AmethystDnsTest { @Test fun `invalidate is case-insensitive`() { val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) dns.lookup("example.com") dns.invalidate("EXAMPLE.com") @@ -420,7 +420,7 @@ class AmethystDnsTest { @Test fun `restore lowercases hostnames so subsequent lookups hit`() { val upstream = CountingDns(mapOf("example.com" to listOf(ip("9.9.9.9")))) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) dns.restore( listOf( @@ -436,7 +436,7 @@ class AmethystDnsTest { fun `dirty flag tracks positive writes`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 60_000, positiveTtlJitterMs = 0, @@ -455,7 +455,7 @@ class AmethystDnsTest { @Test fun `failed lookup does not mark cache dirty`() { val upstream = CountingDns(emptyMap()) - val dns = AmethystDns(delegate = upstream) + val dns = SurgeDns(delegate = upstream) assertFalse(dns.isDirty()) runCatching { dns.lookup("missing.example") } @@ -471,7 +471,7 @@ class AmethystDnsTest { } val syncRefresh = Executor { it.run() } val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 1, positiveTtlJitterMs = 0, @@ -499,7 +499,7 @@ class AmethystDnsTest { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4")))) val rejecting = Executor { throw RejectedExecutionException("test") } val dns = - AmethystDns( + SurgeDns( delegate = upstream, positiveTtlMs = 1, positiveTtlJitterMs = 0,