nic-6443 opened a new pull request, #13917:
URL: https://github.com/apache/apisix/pull/13917

   ### Description
   
   A write made to etcd while APISIX is starting can be lost, permanently and 
without a log line.
   
   The configuration is read once in `init_by_lua` and the snapshot is kept in 
`loaded_configuration`. Each resource type takes its own entry when it 
registers, and `core.config.new()` removes the entry as it does so. When the 
first worker starts watching, `do_run_watch()` took the start revision from 
`next(loaded_configuration)` — whatever entry was still there.
   
   Once every preloaded type has registered, there is nothing there. `rev` 
stays 0, the code falls through to re-reading the current revision with 
`get(prefix)`, and the watch starts from *now* rather than from the snapshot. 
Anything written in between is never delivered to that worker, and nothing says 
so — the existing `invalid or missing X-Etcd-Index header` warning cannot fire 
in this case, because it only runs when an entry was found.
   
   What keeps this from being permanently broken today is a coincidence: the 
http worker registers every type in `HTTP_ETCD_DIRECTORY` except 
`/stream_routes`, and the stream worker registers every type in 
`STREAM_ETCD_DIRECTORY` except `/ssls`, so one entry always survives. That 
balance is not a design — it shifts with the subsystem, with `enable_admin` 
(which decides whether `/plugins` is registered), and with which plugins are 
loaded (`/protos` comes from `grpc-transcode`). Registering one more type, or 
running against an etcd where one of those directories was never created, is 
enough to turn the loss on.
   
   The fix records the revision where it is produced. 
`init_loaded_configuration()` performs a single `readdir`, and 
`create_formatter()` stamps every entry it stores with that one response's 
headers, so there is exactly one revision to keep and no ambiguity about which 
entry to read it from. It is kept even when the range came back empty, since 
the snapshot is still a point in time worth watching from. The `rev == 0` 
fallback stays for the case where no configuration was preloaded at all.
   
   The header check moves to the read, where a bad value can actually occur, so 
the warning from #13364 now fires on the path that produces it rather than on a 
copy of it.
   
   ### Tests
   
   `t/core/etcd-watch-start-revision.t`:
   
   - **TEST 1** drives the preload table empty using only configuration — an 
etcd prefix holding a single directory, so `/routes` is the one type with 
anything to consume — then writes a route between the snapshot and the first 
watch, from `extra_init_by_lua`. The route is not in the snapshot, so it can 
only arrive through the watch. It fails on `master` with `route written during 
startup was lost` and passes with the fix, deterministically in both directions.
   - **TEST 2** covers the `rev == 0` fallback through 
`disable_sync_configuration_during_start`, where there is no snapshot and so no 
revision to watch from.
   
   `t/core/config_etcd.t` TEST 15 is removed. It injected an entry with empty 
headers directly into `loaded_configuration` to reach the warning; 
`get_format()` sets `X-Etcd-Index` from `header.revision` and fails the read 
when it is missing, so no such entry can be produced and the branch it 
exercised was reachable only from the test itself. TEST 2 above covers the 
fallback that can be reached.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to