Skip to content

The nine-second budget does not start when the request does #15

Description

@sshine

From docs/FINDINGS.md. Found by running the Go provider this replaces, and correlated
against this codebase rather than acted on. Sources: external-dns-desec-provider#26,
a year of production fixes, its review, and sshine/external-dns-desec-provider#2,
another operator's account of the same failure from a four-cluster deployment.
This is not a decision.

This corrects the diagram at the top of apply.rs.

The mechanism

That diagram says 15s client, 12s router layer, 9s handler deadline, 4s per attempt. Two waits sit
outside it.

The gate.

pub async fn apply(&self, changes: &Changes) -> ApplyOutcome {
    let _gate = self.gate.lock().await;                    // unbounded
    ...
    let timed_out = tokio::time::timeout(DEADLINE, self.write_all(...)).await.is_err();

The clock starts after the mutex is acquired. If external-dns overlaps two reconciles — which is
the only reason the gate exists — the second request waits an unmetered amount of time and then
grants itself a fresh nine seconds. Only the router's 12s layer bounds the total, and when that
fires the client gets the generic remapped 503 rather than the composed one with a real
Retry-After.

The pacing wait. desec::Client's timeout(4s) is set on the reqwest client, so it covers
send() and the body read. Limiter::acquire runs before that, inside the same execute call,
and may sleep up to max_rate_limit_wait. The innermost bound is 2 + 4 = 6 seconds, not 4.

In practice the 2s is only ever spent after a throttle: one bulk PATCH per zone per gated apply
never fills a 2/s per-domain window, and eight concurrent zones do not trouble user's
2000/day. So the pacing wait is reachable exactly when a penalty is live, which is exactly when
the handler is already slow. CodeRabbit raised the same arithmetic against the Go transport, where
it was worse — there the sleep was inside http.Client.Timeout and ate the budget rather than
extending it.

The part that costs something

When the 12s layer fires, or external-dns hangs up, or the pod drains, the whole apply future is
dropped. write_all's JoinSet goes with it, aborting the in-flight PATCHes, and:

  • the Arc<Mutex<Vec<_>>> of per-zone outcomes is dropped, so which zones succeeded is unknown;
  • store.invalidate is never called for the zones that were attempted;
  • nothing is recorded — including apply_duration_seconds, whose stated purpose is that "a
    near-miss on the 15s client timeout is visible before it becomes an outage". The samples it
    loses are precisely the near-misses.

The snapshot damage is self-healing, which is worth stating because it bounds how much this
matters: a PATCH that landed moves the zone's touched, and needs_relist compares
server-supplied values, so the next tick re-reads the zone whether or not we invalidated it. What
does not heal is the metric. What is merely wasteful is a duplicate write, if external-dns replans
before the next tick.

Where we stand

Fixing the gate ordering is two lines — start the clock at handler entry, or wrap the acquire in
the deadline — and it makes the diagram true rather than aspirational. Whether the dropped-future
case deserves defending is a genuine question: a Drop guard or a detached recorder is more
machinery than a self-healing failure justifies, and the honest answer may be to amend the comment
instead.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfindingRecorded in docs/FINDINGS.md

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions