lordcheng10 opened a new pull request, #13971:
URL: https://github.com/apache/apisix/pull/13971
### Description
Fix a Nacos discovery refresh bug where a per-service query failure could
delete that service's last successful cache entry, even though the service
was
still referenced by APISIX configuration.
In `apisix/discovery/nacos/init.lua`, `fetch_full_registry` builds the set of
service names from the per-host query result and then deletes every cached
key
that is not in that set. The set was only populated *after* an instance-list
query succeeded, so a service whose query failed was missing from it and was
treated exactly like a service that had been removed from APISIX
configuration.
With two Nacos-discovered services A and B: a refresh where A succeeds and B
times out (or returns a network/HTTP error) leaves B out of the name set. The
cleanup loop then sees B in the in-use set but not in the name set, and
deletes
B's last successful snapshot from the `nacos` shared dict. Subsequent
requests
to 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, and only the failing service loses its cache.
A failed query means the latest state is unknown, not that the service has
been
removed from APISIX configuration. This PR makes that distinction explicit:
- Rename `service_names` to `configured_services` in
`apisix/discovery/nacos/client.lua`, and document the new contract in the
`fetch_from_host` docstring.
- Compute the cache key and set `configured_services[key] = true` **before**
calling the instance-list API, so a service is reported as configured
regardless of whether its query succeeds.
- On a query error, keep logging `get_url: <path> err: <error>` and skip only
that service's cache update. The cleanup loop in `fetch_full_registry` now
compares against `configured_services`, so the previous snapshot survives.
- Delete a cached key only when it was configured in the preceding refresh
but
is absent from the current `configured_services` set, i.e. the service was
actually removed from APISIX configuration.
The change is limited to the error path. The behavior for a successful
response with an empty `hosts` list is intentionally unchanged from 3.13.0:
an
empty instance list is still written as an empty node set. Confirming that
semantics is desired is left as a separate question, as this PR only
addresses
the failure path.
#### Which issue(s) this PR fixes:
Fixes #13931
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not,
please discuss on the [APISIX mailing
list](https://github.com/apache/apisix/tree/master#community) first)
### Additional notes
No documentation update was made: this change adds no configuration option,
endpoint or documented behavior, it only stops the discovery cache from being
dropped when a refresh query fails.
Tests added to `t/discovery/nacos2.t`:
- a partial refresh failure keeps the failed service's cached nodes and still
updates the service that succeeded;
- a service removed from the configuration is still cleaned up on the next
refresh;
- the unchanged successful-but-empty `hosts` path;
- the unchanged total-outage path, where no cleanup runs at all.
Reviewer note on verification: `t/discovery/nacos2.t` needs OpenResty plus
the
Nacos and etcd test containers, which were not available in the environment
used to prepare this patch. The change was validated there with a Lua syntax
check and a behavioral harness that replayed the refresh sequence against
both
the 3.13.0 code and this patch (3.13.0 drops the failed service's entry; the
patch retains it while still deleting entries for services no longer
referenced). CI is expected to run the full test file.
--
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]
