nic-6443 opened a new pull request, #14014: URL: https://github.com/apache/apisix/pull/14014
### Description Consul discovery's `connect()` spawns a catalog watcher and a health watcher, waits for the first one to return and then kills both. The watchers used `resty.consul`, which reads the response body through resty.http, and resty.http reads a body in a child coroutine. If the watcher that gets killed is still reading its body (typically the health watch streaming a large `/v1/health/state/any` response while the catalog watch returns first), `ngx.thread.kill` does not cancel the read pending in that child coroutine. When the read completes later, it resumes `connect()` at whatever it is waiting on at that moment, usually the status-line receive of the following catalog fetch. That fetch then fails with: ``` fetch_services_from_server(): connect consul: ... by sub url: /catalog/services, got catalog result: ".../resty/http.lua:479: bad argument #1 to 'str_sub' (string expected, got boolean)" ``` and the round is retried with backoff. It is easy to hit when both indexes change together and the health response is large. The watchers only need the status and the `X-Consul-Index` header. This PR makes them send the blocking query with resty.http `request()`, which does all of its I/O (connect, send, status line, headers) on the calling coroutine, and close the connection without reading the body. A killed watcher therefore never leaves a pending operation behind in a child coroutine. The `connect()` loop itself (spawn, wait, kill, re-arm, retry/backoff, non-keepalive mode) is unchanged, and the query parameters, token header and timeouts are the same as before. Behavior notes: - watcher connections are closed after each query instead of going back to the keepalive pool, so each watch round opens two new connections to Consul; - when a watch fails, the error log now carries the status and headers of the response but no longer its body. The new test `t/discovery/consul-watch-race.t` runs a mock Consul that bumps both indexes together, answers the catalog watch shortly after the health watch has started streaming a slow chunked body, and delays the non-blocking catalog read so that the health body finishes while the fetch is waiting. Before this change the test fails with the `got boolean` error and the route returns 503; after it, the services are fetched and the route returns 200. #### Which issue(s) this PR fixes: N/A ### 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) -- 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]
