From fe7b0b39ac389985edccdd90839b7b1f2216853c Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 20 Jul 2026 15:33:21 +0800 Subject: [PATCH] fix(etcd): advance the watch revision only on in-stream progress #12514 sampled the global etcd revision with an out of band readdir before each watch, and on a watch timeout it moved watch_ctx.rev up to that sample. A timeout only means "no bytes for watch_timeout seconds", which does not distinguish an idle prefix from a watch stream that established and then died silently. In the latter case etcd had already written the pending events into the dead stream, and skipping to the sampled revision drops them for good: the worker keeps a stale configuration until the key is written again or the worker restarts, with no error logged and no reconciliation path. #13067 reproduces this against master. Create the watch with progress_notify instead and only advance the revision on responses read from the stream itself. etcd sends progress notifications only to fully synced watchers, so the revision one carries is a server side guarantee that every event up to it was already delivered on that same stream, and a dead stream simply stops advancing the revision. The out of band readdir is removed, along with the RPC it cost on every watch cycle. Trade-off: notifications are sent every 10 minutes by default while an idle watch stream only lives for watch_timeout (50s), so under a default etcd the revision no longer moves while the prefix is idle and behaviour falls back to what it was before #12514 - a compaction cancels the watch and triggers one full reload. Operators who want to keep #12514's benefit can set --experimental-watch-progress-notify-interval below watch_timeout; this is documented in the FAQ and configured for the CI etcd. Paying for a bounded, self-healing reload beats silently serving a stale configuration forever. The progress notification branch has to run before the "smaller revision" check: on an idle prefix watch_ctx.rev is the last event revision plus one while a notification carries the current store revision, so it is one lower and would otherwise be read as an etcd restart and force a resync on every notification. TEST 14 asserted the removed behaviour and now asserts its absence; TEST 16 covers the notification path and checks that events are still delivered after a notification moved the revision. --- apisix/core/config_etcd.lua | 45 +++++++++++------- ci/pod/docker-compose.common.yml | 3 ++ docs/en/latest/FAQ.md | 10 ++++ t/core/config_etcd.t | 82 +++++++++++++++++++++++++++++--- 4 files changed, 117 insertions(+), 23 deletions(-) diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua index a56a221b6a18..005668919881 100644 --- a/apisix/core/config_etcd.lua +++ b/apisix/core/config_etcd.lua @@ -197,17 +197,17 @@ local function do_run_watch(premature) opts.need_cancel = true opts.start_revision = watch_ctx.rev - -- get latest revision - local res, err = watch_ctx.cli:readdir(watch_ctx.prefix .. "/phantomkey") - if err then - log.error("failed to get latest revision, err: ", err) - end - local latest_rev - if res and res.body and res.body.header and res.body.header.revision then - latest_rev = tonumber(res.body.header.revision) - else - log.error("failed to get latest revision, res: ", json.delay_encode(res)) - end + -- Ask etcd to send progress notifications, i.e. watch responses without + -- events. etcd only sends them to watchers that are fully synced, so the + -- revision they carry is an in-stream guarantee: every event up to that + -- revision has already been delivered on this very stream. They are what + -- keeps start_revision fresh while the watched prefix is idle. Sampling + -- the latest revision out of band instead, as we used to, cannot tell an + -- idle prefix apart from a stream that silently died, and skips the events + -- such a stream never delivered (#13067). + -- The notification interval is a server side setting, + -- --experimental-watch-progress-notify-interval, 10 minutes by default. + opts.progress_notify = true log.info("restart watchdir: start_revision=", opts.start_revision) @@ -228,12 +228,6 @@ local function do_run_watch(premature) then log.error("wait watch event: ", err) end - if err == "timeout" then - if latest_rev and watch_ctx.rev < latest_rev + 1 then - watch_ctx.rev = latest_rev + 1 - log.info("etcd watch timeout, upgrade revision to ", watch_ctx.rev) - end - end cancel_watch(http_cli) break end @@ -303,6 +297,23 @@ local function do_run_watch(premature) break end + -- A watch response without events is a progress notification. etcd + -- only sends it once this watcher is fully synced, so every event up + -- to res.result.header.revision has already been delivered on this + -- stream and the next start_revision can safely move past it. + -- This must come before the "smaller revision" check below: on an idle + -- prefix watch_ctx.rev is the revision of the last event plus one, + -- while a notification carries the current store revision, so rev is + -- watch_ctx.rev - 1 and would be mistaken for an etcd restart. + local events = res.result.events + if not events or #events == 0 then + if rev >= watch_ctx.rev then + watch_ctx.rev = rev + 1 + log.info("etcd progress notify, upgrade revision to ", watch_ctx.rev) + end + goto watch_event + end + if rev < watch_ctx.rev then log.error("received smaller revision, rev=", rev, ", watch_ctx.rev=", watch_ctx.rev,". etcd may be restarted. resyncing....") diff --git a/ci/pod/docker-compose.common.yml b/ci/pod/docker-compose.common.yml index 50d8004c53d1..f8c0f1a5f72f 100644 --- a/ci/pod/docker-compose.common.yml +++ b/ci/pod/docker-compose.common.yml @@ -54,6 +54,9 @@ services: - ci/pod/etcd/env/common.env environment: ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379 + # send watch progress notifications often enough to be observed within + # the lifetime of a watch stream, whose default timeout is 50s + ETCD_EXPERIMENTAL_WATCH_PROGRESS_NOTIFY_INTERVAL: "3s" ports: - "2379:2379" - "2380:2380" diff --git a/docs/en/latest/FAQ.md b/docs/en/latest/FAQ.md index 96c3f3a0c542..b8b72fe75f57 100644 --- a/docs/en/latest/FAQ.md +++ b/docs/en/latest/FAQ.md @@ -317,6 +317,16 @@ If there is no change in the directory being monitored, the process will be bloc If there are changes in the directory being monitored, etcd will return this new data within milliseconds and Apache APISIX will update the cache memory. +## Why should I lower the etcd watch progress notification interval? + +Apache APISIX creates its watch with `progress_notify` enabled. etcd then sends periodic notifications on the watch stream telling APISIX which revision the stream is synced up to, which lets APISIX keep the start revision of the next watch fresh even while nothing changes under the watched prefix. A fresh start revision means the watch is not cancelled with a `compacted` error after an etcd compaction, and APISIX does not have to reload the whole configuration to recover. + +etcd sends those notifications every 10 minutes by default, while an idle APISIX watch stream only lives for `deployment.etcd.watch_timeout` (50 seconds by default), so with the default etcd settings APISIX rarely sees one. If your etcd compacts frequently and you want to avoid the resulting full reloads, set the notification interval below `watch_timeout` on the etcd side: + +```shell +etcd --experimental-watch-progress-notify-interval=30s +``` + ## How do I customize the Apache APISIX instance id? By default, Apache APISIX reads the instance id from `conf/apisix.uid`. If this is not found and no id is configured, Apache APISIX will generate a `uuid` for the instance id. diff --git a/t/core/config_etcd.t b/t/core/config_etcd.t index 9ccac862f441..0047ca55bb55 100644 --- a/t/core/config_etcd.t +++ b/t/core/config_etcd.t @@ -521,7 +521,7 @@ main etcd watcher initialised, revision= -=== TEST 14: watch revision should be upgraded when timeout occurs +=== TEST 14: watch revision must not be upgraded when the watch times out --- yaml_config deployment: role: traditional @@ -547,7 +547,8 @@ nginx_config: return end ngx.sleep(2) - -- we will assert 4 lines of revision upgrade log because we have one worker and one privileged agent + -- write outside the watched prefix so that the global revision + -- moves on while the watch itself stays idle and keeps timing out for i = 1, 2 do local _, err = etcd_cli:set("/apache", "apisix") if err then @@ -563,10 +564,8 @@ nginx_config: GET /t --- response_body passed ---- grep_error_log eval -qr/etcd watch timeout, upgrade revision to/ ---- grep_error_log_out eval -qr/(etcd watch timeout, upgrade revision to\n){2,}/ +--- no_error_log +etcd watch timeout, upgrade revision to @@ -615,3 +614,74 @@ passed qr/invalid or missing X-Etcd-Index header/ --- grep_error_log_out eval qr/(invalid or missing X-Etcd-Index header\n){1,}/ + + + +=== TEST 16: watch revision is upgraded by etcd progress notifications +# relies on ETCD_EXPERIMENTAL_WATCH_PROGRESS_NOTIFY_INTERVAL=3s in the CI etcd, +# so that notifications arrive well within the default 50s watch timeout +--- yaml_config +deployment: + role: traditional + role_traditional: + config_provider: etcd + admin: + admin_key: null + etcd: + host: + - "http://127.0.0.1:2379" + prefix: /apisix +--- extra_yaml_config +nginx_config: + worker_processes: 1 +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local t = require("lib.test_admin").test + + -- keep the watch stream idle long enough to be notified a few times + ngx.sleep(10) + + -- events must still be delivered after the notifications moved the + -- revision forward + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + } + }]] + ) + if code >= 300 then + ngx.status = code + return + end + ngx.say(body) + + ngx.sleep(0.5) + + local httpc = http.new() + local res, err = httpc:request_uri( + "http://127.0.0.1:" .. ngx.var.server_port .. "/hello") + if not res then + ngx.log(ngx.ERR, err) + return + end + ngx.print(res.body) + } + } +--- timeout: 20 +--- request +GET /t +--- response_body +passed +hello world +--- grep_error_log eval +qr/etcd progress notify, upgrade revision to/ +--- grep_error_log_out eval +qr/(etcd progress notify, upgrade revision to\n){1,}/