apache / apache/apisix

bug: retain Nacos service cache when a per-service refresh query fails

Open
#13,931 3 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.