github-actions[bot] commented on code in PR #63363:
URL: https://github.com/apache/doris/pull/63363#discussion_r3338363867
##########
be/src/util/dns_cache.cpp:
##########
@@ -97,26 +151,57 @@ Status DNSCache::_update(const std::string& hostname) {
cache[hostname] = real_ip;
LOG(INFO) << "update hostname " << hostname << "'s ip to " << real_ip;
Review Comment:
This insert can reintroduce a hostname that the refresh thread has just
evicted. A real caller can enter `get()` while the host is cached, miss DNS
inside `_resolve_hostname()`, then the refresh thread reaches the threshold and
calls `_erase()` while DNS resolution is still blocked. `_resolve_hostname()`
returns the previously captured `cached_ip` with `failures == 0`, and
`_update()` then sees `cache.find(hostname) == end` here and writes the stale
IP back into `cache`. That defeats the purpose of eviction and lets brpc
callers continue using the removed backend address. The new race test
acknowledges this by calling `_resolve_hostname()` directly to avoid the
`_update()` reinsert, but production callers go through `_update()`. Please
make `_update()` distinguish resolved IPs from fallback cached IPs and avoid
inserting the fallback when the entry disappeared during resolution, or
otherwise re-check the eviction state before writing.
--
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]
