github-actions[bot] commented on code in PR #69423:
URL: https://github.com/apache/doris/pull/69423#discussion_r4345947506
##########
fe/fe-core/src/main/java/org/apache/doris/common/DNSCache.java:
##########
@@ -57,7 +59,26 @@ public void start() {
* @return The IP address for the given hostname.
*/
public String get(String hostname) {
- return cache.computeIfAbsent(hostname, this::resolveHostname);
+ String cachedIp = cache.get(hostname);
+ if (cachedIp != null && !refreshPending.contains(hostname)) {
+ return cachedIp;
+ }
+ return cache.compute(hostname, (host, currentIp) -> {
+ // Claim before DNS I/O so an invalidation arriving during this
lookup is
+ // retained for the next get(). Concurrent lookups of this host
serialize here.
+ boolean needsRefresh = refreshPending.remove(host);
Review Comment:
[P2] Keep warm readers behind an in-flight forced DNS refresh
After a restart heartbeat marks cached IP A pending and clears the proxy
maps, the first query removes this marker and blocks resolving replacement IP
B. A second query admitted after that heartbeat sees cached A with no marker at
lines 62-64, bypasses `compute`, and creates a channel to the retired pod after
connection invalidation has finished. Concurrent queries can therefore fail and
exhaust their retries while the forced lookup is still running. The BE cache
has the same early marker removal in `DNSCache::_update()`, allowing concurrent
tablet-writer acquisitions to rebuild a failed channel with A. Keep
refresh-in-flight state separate from newly pending invalidations so warm
readers wait for the forced result, retaining A as fallback only when
resolution fails. Add a populated-cache, two-reader latch test; the
first-lookup invalidation test does not cover this ordering.
##########
be/src/util/dns_cache.cpp:
##########
@@ -210,6 +225,7 @@ std::string DNSCache::_resolve_hostname(const std::string&
hostname, bool* is_fr
void DNSCache::_evict_locked(const std::string& hostname) {
cache.erase(hostname);
+ _refresh_pending.erase(hostname);
Review Comment:
[P2] Preserve newer refresh requests across background eviction
An old channel's network-fault callback can call `invalidate()` while a
background lookup has captured NXDOMAIN but has not published it. If that
lookup reaches `dns_cache_max_consecutive_failures` (for example, with the
supported threshold set to 1), `_update()` preserves the newer pending marker,
but `_refresh_once()` subsequently evicts the entry and this erase discards the
marker before installing a negative-cache tombstone. Even if the replacement
pod is now resolvable, subsequent `get_client()` calls refuse resolution for
the default 60-second TTL; no RPC is sent to trigger another invalidation.
Fence eviction against invalidations received since the lookup began so the
next acquisition can resolve again. Cover this with an authoritative-NXDOMAIN
latch test; the added EAI_AGAIN test cannot reach eviction.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
