Repository navigation
Conversation
A TTL of 0 means the answer may only be used for the transaction in progress and must not be cached (RFC 1035 section 3.2.1). The new DNS client passed that zero straight to mlcache, which reads a zero TTL as "never expires": it drops the TTL for the LRU and marks the shm entry with NO_TTL_FLAG. A single answer with TTL 0, which a caching resolver returns routinely, therefore pinned the name in kong_dns_cache for the life of the node, and Kong kept sending traffic to addresses that had changed. Return such answers with a negative TTL, the convention resolve_callback already uses for cache_only, so mlcache skips both of its caches and the name is resolved again on the next request. Every other TTL is unaffected, unlike the resolver_valid_ttl workaround, which overrides the TTL of every answer. * add cache_ttl() and use it on both return paths of resolve_query_names() * add a regression test to the new DNS client cache spec
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A DNS answer with a TTL of 0 means the answer may only be used for the transaction in progress and must not be cached (RFC 1035 section 3.2.1). The new DNS client passes that zero straight through to mlcache, where a zero TTL is the sentinel for "never expires":
set_lrudrops the TTL (if ttl == 0 then ttl = nil end) and the shm path marks the entry withNO_TTL_FLAG. One answer with TTL 0, which a caching resolver returns routinely, therefore pins the name inkong_dns_cachefor the life of the node, across every worker and acrosskong reload, and Kong keeps sending traffic to addresses that have changed. This is what #15000 reports against an AWS ALB, where the pinned addresses were withdrawn and the requests turned into 504s.The fix returns such answers with a negative TTL, which is the convention
resolve_callbackalready uses forcache_only, so mlcache skips both of its caches and the name is resolved again on the next request. Every other TTL keeps its value, which is the difference from theresolver_valid_ttlworkaround: that one overrides the TTL of every answer, so it also stops honouring the zone for names that publish a perfectly good TTL.The legacy client does not have this failure mode because it never hands a bare zero to a cache that reads zero as forever: it inserts with
lru_ttl = ttl + staleTtl, so a zero-TTL record lands already stale and gets refreshed.How I verified it. I could not run the busted suite in my environment (no
bustedrock available for the Lua version in the container I had), so I exercised the same code path with a standalonerestyharness that mocksresty.dns.resolver.query, resolves a name whose record carriesttl = 0, replaces the mocked record with a different address, and resolves again:The test added to
spec/01-unit/30-new-dns-client/03-old_client_cache_spec.luaasserts exactly that, and it is the assertion on the number of queries that fails on master.Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
Discussion: #15000
Closes #15021