Skip to content

Pace certificate renewal retries instead of blocking on one long job#394

Open
Socialpranker wants to merge 1 commit into
caddyserver:masterfrom
Socialpranker:fix-renewal-retry-backoff
Open

Pace certificate renewal retries instead of blocking on one long job#394
Socialpranker wants to merge 1 commit into
caddyserver:masterfrom
Socialpranker:fix-renewal-retry-backoff

Conversation

@Socialpranker

Copy link
Copy Markdown

Fixes caddyserver/caddy#7843.

What's going on

queueRenewalTask submits a single job to jm that internally calls doWithRetry, which keeps retrying for up to 30 days with a growing backoff (up to 6 hours between attempts). While that job is still running, jm.Submit's dedup-by-name silently drops every subsequent attempt to queue the same renewal, because the name is only removed once the job returns.

The periodic maintenance tick (RenewCheckInterval, 10 minutes by default) keeps calling queueRenewalTask and logging "certificate expires soon; queuing for renewal" every time, which looks like it's doing something every 10 minutes — but it isn't. The actual next attempt only happens whenever the already-running job's internal sleep wakes up, which by that point in the backoff schedule can be hours away.

That's what the linked issue is describing: a cert failed to renew because of a temporary DNS problem, the DNS got fixed, and the cert still sat there unrenewed for a long time because the collector was asleep inside a 6-hour backoff step from before the DNS was fixed.

What I changed

Moved the backoff schedule out of the job and into queueRenewalTask itself:

  • queueRenewalTask now checks a small per-domain backoff tracker (renewalBackoff in async.go) before submitting anything. If the domain failed recently and isn't due for a retry yet, it just returns without touching jm at all.
  • The job it submits now calls a new renewCertOnce (one attempt, no internal loop) instead of RenewCertAsync (which loops via doWithRetry). So the job returns right after a single attempt, and the dedup slot in jm is only held for as long as that one attempt takes, not for the whole multi-day retry campaign.
  • On failure, the backoff tracker schedules the next allowed attempt using the same retryIntervals that doWithRetry already uses, so the pacing behavior (fast retries at first, growing to 6h) is preserved — it's just driven by the maintenance tick now instead of a sleeping goroutine.
  • On success (or once the backoff budget of maxRetryDuration runs out), the tracker forgets the domain.

RenewCertSync and RenewCertAsync themselves are untouched. Nothing else that calls them (on-demand handshakes, manageOne, forced renewals) changes behavior.

One side effect worth flagging: the "certificate expires soon; queuing for renewal" info log no longer fires on every maintenance tick while a domain is in its backoff window — it only fires right before an actual attempt. I think that's more honest than the old behavior (which logged that line every 10 minutes regardless of whether anything was actually attempted), but wanted to call it out since someone might be grepping for that line.

Testing

  • TestRenewalBackoffSchedule — unit test on the new backoff tracker directly, using synthetic timestamps (no real sleeping).
  • TestQueueRenewalTaskPacesRetriesItself — exercises queueRenewalTask against a fake issuer that always fails, calling it twice back-to-back. First call attempts and fails; the second call (still within the backoff window) makes no attempt at all, confirmed via the fake issuer's call count and the log output. I also temporarily removed the backoff check to confirm this test actually fails without the fix, then put it back.
  • go build ./..., go vet ./..., gofmt -l ., go test -race ./... all clean, no regressions in the existing suite.

Assistance Disclosure: I used Claude Code to help investigate and implement this — it dug up the root cause (the job-manager dedup + doWithRetry interaction) and drafted the fix, I reviewed the diff and ran the tests myself before opening this.

… job

A certificate that fails to renew (e.g. because DNS is temporarily
broken) used to get stuck: queueRenewalTask submitted a single job to
the jobManager that internally retried for up to 30 days with a
growing backoff. While that job was in flight, every subsequent
maintenance tick tried to queue the same job again, but jobManager's
dedup-by-name silently dropped it. So once the backoff grew to hours,
a fixed DNS record wouldn't get picked up until the sleeping job woke
up on its own schedule, sometimes many hours later, even though the
logs made it look like a new attempt was queued every 10 minutes.

This moves the backoff schedule out of the job and into
queueRenewalTask itself. Each job now makes a single renewal attempt
and returns right away, so it never occupies the dedup slot for more
than one attempt. The periodic maintenance tick is now what actually
paces retries, gated by the same retryIntervals schedule doWithRetry
already used.

RenewCertSync/RenewCertAsync are unchanged and still used by
manageOne, on-demand handshakes, and forceRenew.
@Sn0wCrack

Copy link
Copy Markdown

Something like this is really needed for anyone who handles a large number of domains on a single Caddy instance.

Internally we use a thin cache service that we use as Caddy's ask endpoint that basically does exactly this. Otherwise the server effectively deadlocks itself attempting to provision SSL certificates with ACME providers.

@mholt

mholt commented Jul 19, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. Been out with some family things for a bit, I will try to get to this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ARI unable to handle negative remaining values

3 participants