fix(etcd): advance the watch revision only on in-stream progress

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....")

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"

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.

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,}/