Skip to content

DoH circuit breaker counts endpoint-switch cancellations as failures and resets on any success #760

Description

@kasnder

Found in a correctness review of the DNS path (the malformed-query poisoning variant is already fixed by #753; these accounting gaps remain).

Endpoint switches convert in-flight queries into failure counts. DnsOverHttpsClient.getInstance(context, endpoint) (DnsOverHttpsClient.java:111-119) shuts the previous client down asynchronously via dispatcher().cancelAll() (:158-166). Pool threads still inside resolve() on the old client fail immediately, and DnsProxyServer counts every null resolve() as a DoH failure (UDP DnsProxyServer.java:244-272, TCP :377-387). Switching endpoints under load can add several instant failures and trip the breaker against the new, healthy endpoint — SERVFAIL for every app for 60s right after a settings change.

Any single success resets the counter. A successful response sets dohFailures = 0 and clears circuitOpenUntil (DnsProxyServer.java:252-253). A persistently dead endpoint that occasionally serves an HTTP-cached answer may therefore never trip the breaker, stretching full-SERVFAIL windows indefinitely.

One global counter conflates paths. dohFailures is shared by the UDP and TCP handlers, so failures from one transport mask or trigger the other.

Suggested direction: don't count cancellations caused by our own shutdown(); replace reset-on-success with a time-windowed failure rate; optionally per-path counters.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions