fix(etcd): do not advance the watch revision on a timeout

[AlinsRan]

Fixes #13067

Deletes the watch-timeout revision jump added in #12514. It infers the delivery progress of the watch stream from the health of a different connection, and when that inference is wrong the worker loses configuration silently and permanently.

Net effect: 18 lines deleted, 7 added — all of them comments.

The problem

config_etcd.lua samples the global etcd revision on a separate connection before each watch, then jumps to it if the watch times out:

-- before every watch: sample the latest revision on another connection
local res, err = watch_ctx.cli:readdir(watch_ctx.prefix .. "/phantomkey")
local latest_rev = tonumber(res.body.header.revision)

-- ... and when the watch times out:
if err == "timeout" then
    if latest_rev and watch_ctx.rev < latest_rev + 1 then
        watch_ctx.rev = latest_rev + 1        -- skip straight to the sample
    end
end

A timeout only means "no bytes arrived for watch_timeout seconds". That is equally true of an idle prefix and of a stream that established and then died silently — and the code cannot tell them apart:

t0   watch established, start_revision = R
t1   readdir samples latest = A            (on a different, healthy connection)
t2   watch stream dies silently            (no FIN, no RST — nothing observable)
t3   etcd writes the events for (R, A] into that dead stream
t4   50s elapse, zero bytes read           -> "timeout"
t5   watch_ctx.rev = A + 1                 -> everything in (R, A] is skipped

It never heals

sync_data only sets need_reload on compacted / restarted. After the jump the start revision is fresh, so compaction never fires either — nothing will ever go back for the skipped range.

The worker keeps serving a stale configuration until that key is written again or the worker restarts:

• deleted routes keep routing traffic
• new routes and certificates never take effect
• the only trace is a single info-level log line

Each worker has its own watcher, so some workers can be stale while others are current.

The fix

Delete the sampling and the jump. Nothing is added in their place.

Reproducing it

The failure needs an asymmetric fault: the watch stream must go dark while short requests keep working. (If the whole link fails, the sampling readdir fails too, latest_rev is nil, and the existing guard already prevents the jump.)

Option 1 — iptables, no code needed

# 1. start etcd + APISIX, wait for the watch to establish
grep "restart watchdir: start_revision=" logs/error.log

# 2. find the watch connection (it is the long-lived one; short requests use new sockets)
ss -tn 'dst :2379'

# 3. blackhole only that connection — no RST, and range requests are untouched
iptables -I INPUT -p tcp --sport 2379 --dport <WATCH_SPORT> -j DROP

# 4. write a route during the blackhole
curl -X PUT http://127.0.0.1:9180/apisix/admin/routes/blackhole \
  -H "X-API-KEY: $admin_key" \
  -d '{"uri":"/blackhole","upstream":{"nodes":{"127.0.0.1:1980":1}}}'

# 5. wait out watch_timeout (50s), then restore
grep "etcd watch timeout, upgrade revision to" logs/error.log
iptables -D INPUT -p tcp --sport 2379 --dport <WATCH_SPORT> -j DROP

Result: etcdctl get /apisix/routes/blackhole returns the route, but /blackhole stays 404 — across every healthy watch cycle that follows, indefinitely.

Option 2 — selectively silent TCP proxy (what I used)

Put a proxy between APISIX and etcd, point deployment.etcd.host at it, and have it read the first request line:

• POST /v3/watch → forward the request, forward the response headers, then silently discard the body and hold the connection open (no FIN, no RST)
• anything else (/v3/kv/range, …) → forward normally

Then follow steps 4–5 above.

Real-world equivalents of this shape: NAT/conntrack reaping long-lived connections while short ones pass, conntrack tables being flushed (kube-proxy restart, iptables -F, CNI rebuild), asymmetric packet loss, and an etcd-side watch stall with range still healthy.

Consequence, stated up front

This restores pre-#12514 behaviour. On a shared etcd where this cluster's prefix is idle, unrelated writes advance the global revision, a compaction cancels the watch with compacted, and APISIX does one full readdir reload to recover — the CPU spike reported in #12167, at compaction frequency.

That is a deliberate trade. A bounded, self-healing, observable reload is a better failure mode than unbounded, silent, permanent configuration drift. A gateway that reads etcd once more is a slower gateway; a gateway that loses configuration is a wrong one.

The regression is narrower than it sounds:

Deployment	Affected?
Dedicated etcd	No — when this cluster is idle nothing else advances the global revision either, so compact_revision never overtakes watch_ctx.rev
Prefix with any traffic of its own	No — every event advances the revision
Shared etcd (multiple clusters, or etcd shared with other apps) and this prefix idle	Yes

This is not a bare revert of #12514. #12167 gets a real fix in #13772, which attacks the cost rather than trying to guess what a silent stream delivered: a compacted recovery currently re-validates every item and bumps conf_version unconditionally (load_full_data sets changed on any valid item), rebuilding the whole radixtree even when nothing changed. Reusing items whose modifiedIndex is unchanged makes an idle reload nearly free — and covers etcd restarts and restarted too, which #12514 never did.

The two are deliberately separate PRs: this one is a correctness fix that should be backported, #13772 is a performance change with its own risk surface. Neither blocks the other.

Alternatives considered

Verify the (rev, latest] range is event-free before jumping — killed by deletes

Worth spelling out, because it looks like it should work.

RangeRequest does have min_mod_revision / max_mod_revision, so the range check is expressible. Non-atomicity is not the problem either: an event landing after the check carries a revision above latest, so the next watch starting at latest + 1 still receives it.

What kills it is deletion. A key deleted inside (rev, latest] is not in the range result at all — the check comes back empty, the jump is taken, and the delete event is lost for good. Deletes are the worst case to lose: a removed route keeps serving traffic. No range-based verification can see them.

Use progress_notify and advance only on in-stream notifications — correct, but inert by default

etcd only sends progress notifications to fully synced watchers, so the revision one carries is a server-side guarantee that everything up to it was already delivered on that same stream. Correct, and it would preserve #12514's benefit.

The catch: the progress ticker is created per stream and defaults to 10 minutes, while an idle APISIX watch stream only lives for watch_timeout (50s). Under a default etcd it never fires. The feature would be inert unless the operator also sets --watch-progress-notify-interval (--experimental-watch-progress-notify-interval before etcd 3.6; the flag itself only exists since 3.4.11).

Carrying an ordering-sensitive new branch, a lua-resty-etcd version floor and an FAQ entry for something that does nothing out of the box is not worth it next to making the reload cheap. Can be revisited on its own merits.

Tests

TEST 14 asserted exactly the removed behaviour — etcd watch timeout, upgrade revision to appearing at least twice. Same topology, inverted assertion: the log must not appear at all. Old test and new test are mirrors, which pins the change in both directions.

The blackhole scenario cannot be expressed in test-nginx (it needs a selectively silent TCP proxy), so it stays the manual procedure above.

Backport

Affects 3.14.0 and later. Should be backported to the 3.14 / 3.15 release branches.