bug: retain Nacos service cache when a per-service refresh query fails
- Dominant language
- Lua
- Stars
- 17.1k
- Forks
- 2.9k
- Avg merge
- 3d 16h
- Merged PRs (30d)
- 63
Description
### Current Behavior
In the Nacos discovery refresh path, a per-service Nacos query failure can be treated as if the service were no longer referenced by APISIX when another service refresh succeeds.
In APISIX 3.13.0, `fetch_full_registry` builds `service_names` only after `get_url` succeeds. If service B fails to refresh, B is absent from `service_names`. The cleanup loop then deletes keys present in `curr_service_in_use` but absent from `service_names`.
This conflates whether a service remains referenced by APISIX configuration with whether its Nacos query succeeded. A transient failure for B can therefore delete B's last successful registry snapshot from the `nacos` shared dict. Subsequent requests for B can fail with `no valid upstream node`, although a usable cached snapshot existed before the refresh.
This is disruptive during a partial Nacos/API/network failure: service A refreshes successfully, service B times out or returns an error, and only B loses its cache.
### Expected Behavior
A Nacos query failure should mean that the latest state is unknown, not that the service has been removed from APISIX configuration.
For a service still referenced by an APISIX route, upstream, service, or stream route, a failed refresh should retain its last successful cache entry. A successful refresh should update that entry normally. A cache entry should be deleted only after the service is no longer referenced by APISIX configuration.
Please also confirm the desired semantics for a successful but empty `hosts` response. This proposal intentionally does not change the 3.13.0 behavior for that case; it only changes the error path.
### Error Logs
No production log is attached. The relevant failure is logged as:
`get_url: err: `
The refresh then continues and the cleanup phase can delete the failed service's prior cache entry.
### Steps to Reproduce
1. Configure two Nacos-discovered services, A and B, and allow an initial refresh to populate both entries in the `nacos` shared dict.
2. On a later refresh, make the instance-list query for A succeed.
3. Make the instance-list query for B fail, for example with a timeout, network failure, or HTTP error.
4. In 3.13.0, B is not added to `service_names`, because that assignment happens only after `get_url` succeeds.
5. The cleanup loop sees B in `curr_service_in_use` but not in `service_names` and deletes B's cached entry.
6. Send a request to an APISIX upstream using B. It may fail because discovery no longer finds B's nodes.
Proposed fix:
- Rename `curr_service_in_use` to `curr_configured_services`.
- Build `configured_services` from APISIX configuration before each Nacos query.
- Calculate the cache key and record `configured_services[key] = true` before calling `get_url`.
- On a query error, log and skip only that service's cache update.
- Delete a cached key only when it was configured in the preceding refresh but is absent from the current `configured_services` set.
This keeps a failed service's previous cache entry available, while a service removed from APISIX configuration is still cleaned up on the next refresh.
### Environment
- APISIX version: 3.13.0
- Discovery type: Nacos
- Relevant source file: `apisix/discovery/nacos/init.lua`
- Cache storage: `lua_shared_dict nacos 10m`
- Reproduced by code-path analysis on the 3.13.0 branch; no operating-system-specific behavior is required.
Contributor guide
Research direction
Read apisix/discovery/nacos/init.lua, starting at fetch_full_registry and the get_url error path. Trace how configured services and cache keys are collected before cleanup, then verify that a failed query skips only its update while a removed configuration still permits deletion. Done means a failed refresh retains the prior cache entry and a successful refresh updates it normally.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- lua
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100