github-actions[bot] commented on code in PR #69423:
URL: https://github.com/apache/doris/pull/69423#discussion_r4345530908
##########
fe/fe-core/src/main/java/org/apache/doris/system/Backend.java:
##########
@@ -881,11 +883,28 @@ public String toString() {
+ ", backendStatus: " + backendStatus;
}
+ /** Refresh FE-local connections before publishing a restarted backend. */
+ public void invalidateLocalConnections() {
+ Env.getCurrentEnv().getDnsCache().invalidate(host);
+ BackendServiceProxy.invalidateForAll(new TNetworkAddress(host,
brpcPort));
Review Comment:
[P2] Skip serving-connection invalidation during checkpoint replay
Checkpoint construction loads an older image and replays historical
heartbeat transitions through `handleHbResponse(..., true)`, so an old backend
recovery or epoch change also reaches this method.
`BackendServiceProxy.Holder.proxies` and `ClientPool.backendPool` are
process-wide: this replay shuts down the serving FE's healthy clients even
though no backend is currently restarting. A result fetch paused between
`getProxy()` and `client.fetchDataAsync()` then starts on a shut-down channel
and fails; a query that has already sent rows cannot retry. Suppress
operational connection invalidation on `Env.isCheckpointThread()` while
retaining it for normal follower replay, and cover checkpoint replay in the
restart tests.
##########
be/src/util/dns_cache.cpp:
##########
@@ -251,6 +272,25 @@ bool DNSCache::_erase_if_still_failing(const std::string&
hostname, uint32_t thr
Status DNSCache::_update(const std::string& hostname, FailureState* out_state,
std::string* out_ip) {
+ std::shared_ptr<std::mutex> refresh_mutex;
+ {
+ std::unique_lock<std::shared_mutex> lock(mutex);
+ auto [it, inserted] = _refresh_locks.try_emplace(hostname);
+ if (inserted) {
+ it->second = std::make_shared<std::mutex>();
+ }
+ refresh_mutex = it->second;
+ }
+ Defer remove_refresh_lock {[&] {
+ std::unique_lock<std::shared_mutex> lock(mutex);
+ // Only this call and the map retain the mutex: no running lookup or
waiter needs it.
+ if (refresh_mutex.use_count() == 2) {
+ _refresh_locks.erase(hostname);
+ }
+ }};
+ // A background lookup that captured IP A must finish before a forced
lookup
+ // publishes B; otherwise its stale fallback (or old DNS answer) can
overwrite B.
+ std::lock_guard<std::mutex> refresh_lock(*refresh_mutex);
Review Comment:
[P2] Reuse completed DNS lookups after waiting for the hostname lock
When several fragment-preparation workers call `get()` for the same uncached
hostname while DNS is timing out, they all pass the cache/negative-cache checks
before the first resolution finishes and queue on this mutex. The first caller
installs a negative-cache entry, but every waiter still calls
`_resolve_hostname()` unconditionally, paying another full resolver timeout in
sequence. The Nth worker can therefore remain blocked for N resolver timeouts
and exhaust the fragment-preparation pool despite the negative TTL. Share the
completed result or recheck positive/negative cache after acquiring the lock,
while preserving explicit forced/background refresh semantics. A concurrent
cold-miss test should cover this ordering.
--
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]
