kong icon indicating copy to clipboard operation
kong copied to clipboard

Warmup/DNS: IPs are resolved and cached at warmup phase only in worker #0

Open marc-charpentier opened this issue 3 years ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Kong version ($ kong version)

2.8.1

Current Behavior

Hello,

When Kong starts, the warmup phase is performed by worker #0 only, supposedly because as entities are cached in shared memory (L2 cache), letting all workers do it would uselessly over-solicitate the database. But if services entities are to be warmed up, "DNS warmup" is also performed, and resolved IP addresses are cached in the worker's individual memory (L1 cache). So, with Kong running several workers (8 on our production nodes), DNS warmup doesn't take place for all workers except worker #0, and IP addresses resolutions are thus performed later, in request context. The impact on Kong latency is small, about 20 ms, but happens once per worker (except #0) and per hostname to resolve.

A solution would be to modify the code so that all other workers may also load, but not cache, services entities and thus perform hostnames resolution. I tested such modification, and the way I implemented it allowed also to modify the plugins iterator in order to let it reload and cache plugins when it is rebuilt, and only in worker #0.

If agreed, I can submit a PR in order to contribute with this little improvement.

Expected Behavior

All workers should resolve services' hostnames during warmup, not only worker #0.

Steps To Reproduce

1. Provision Kong with a service and an associated route.
2. Restart Kong with 8 workers (KONG_NGINX_WORKER_PROCESSES=8 kong restart).
3. Curl the created route 20 times, greping the X-Kong-Latency-Proxy header.
4. In 7 responses (tending to be in the firsts), the X-Kong-Latency-Proxy jumps from 1-2ms to 20-40ms.

Anything else?

I also added logging of the time taken by kong.resty.dns.client:toip call in kong/runloop/balancer/init.lua:306. It ranged from 15 to 18 ms.

marc-charpentier avatar Jun 20 '22 13:06 marc-charpentier

Hello @marc-charpentier,

A solution would be to modify the code so that all other workers may also load, but not cache, services entities and thus perform hostnames resolution

That solution might make Kong more response, but it will result in more requests to the database on init, and also nxm dns requests while kong is booting up and filling the caches. I think a better solution would be to address the problem directly:

entities are cached in shared memory (L2 cache), [...] resolved IP addresses are cached in the worker's individual memory (L1 cache)

Let's consider using the L2 cache for the DNS queries as well. I see two ways to do this:

  • Modifying lua-resty-dns-client so that it accepts an external cache as a parameter to client.new. Then we could initialize the dns client giving it our own L2 cache from kong.
  • Or we could leave lua-resty-dns-client untouched and wrap it with our own cache (kong.dns would wrap lua-resty-dns-client by caches). This is simpler (only one PR needed) but perhaps a bit less elegant.

(I have a nagging concern: is it appropriate to cache DNS queries on L2? I don't know if there's a realistic scenario in which a DNS server can return different information to different workers inside the same Kong node @locao what do you think?).\

also to modify the plugins iterator in order to let it reload and cache plugins when it is rebuilt, and only in worker #0.

That sounds like an unrelated feature (different PR/issue), but maybe I am missing something

kikito avatar Jul 06 '22 13:07 kikito

Hello and thanks for your answers @kikito,

Indeed, I reached the same conclusions about resulting in more database and DNS queries on init, but I also assumed that there were good reasons for caching DNS entries in L1 instead of L2, somehow related to your concern regarding this matter, and that both database and DNS servers would still be able to handle those additional queries. A reason to cache DNS entries in L1 only may be that a same service/target may resolve to different IPs on a round-robin basis, or to several ones at once (letting the client choose the one to use), which is pretty common in the environments where our Kong gateways operate. So, caching a DNS entry in L2 would probably result in all workers proxying a service's traffic toward a single IP whereas several may be available, whereas letting each worker do a DNS query on its own, and cache the entry in L1, would allow to distribute the traffic on several IPs. So, if we still consider using L2 :

  • I'm not sure whether the first option you suggest would need two PRs because client.lua is maintained in lua-resty-dns-client, or because it would be cleaner to separate its modification in kong's copy (kong/resty/dns/client.lua) from the kong/tools/dns.lua's modification required to provide the L2 cache reference in the options table (hoping I understood well).
  • It seems to me kong.dns ends up to be the initialized DNS client itself, and I'm currently not familiar enough with the code to understand what you mean by "wrap lua-resty-dns-client by caches", so do you have an example I could look to ?

About the plugin iterator, you're right, but I thought that the proposed feature relied on one of the modifications I suggested in order to make all workers do DNS resolution. I'll open an other issue on this topic in order to separate it from the current one.

marc-charpentier avatar Jul 08 '22 14:07 marc-charpentier

@marc-charpentier 👏 👏 👏

A reason to cache DNS entries in L1 only may be that a same service/target may resolve to different IPs on a round-robin basis, or to several ones at once (letting the client choose the one to use), which is pretty common in the environments where our Kong gateways operate. So, caching a DNS entry in L2 would probably result in all workers proxying a service's traffic toward a single IP whereas several may be available, whereas letting each worker do a DNS query on its own, and cache the entry in L1, would allow to distribute the traffic on several IPs.

That is exactly why the DNS cache is per worker, and not node-wide. First ever for me not having to explain it 😄 .

Tieske avatar Jul 19 '22 12:07 Tieske

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 03 '22 02:12 stale[bot]