github-actions[bot] commented on code in PR #69423:
URL: https://github.com/apache/doris/pull/69423#discussion_r4374314266
##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -923,6 +924,13 @@ void FragmentMgr::_collect_invalid_queries(
}
}
+ // Stream-load coordinators live on BE and can finish their
transaction
+ // against a new FE master. Losing the originating FE during a
planned
+ // restart must not cancel that load. Timeout/leak checks
above still run.
+ if (q_ctx->get_query_source() == QuerySource::STREAM_LOAD &&
+ _exec_env->cluster_info()->in_graceful_shutdown()) {
Review Comment:
[P2] Preserve the transaction before continuing across master loss
For an ordinary one-phase Stream Load in shared-nothing mode, restart master
A and elect B while the upload is still running. `loadTxnBeginImpl()` creates a
`BACKEND_STREAMING` transaction, and both
`DatabaseTransactionMgr.persistTransactionState()` and
`enqueueTransactionState()` deliberately skip its `PREPARE` state. B therefore
has no record of the original transaction. This new exemption lets the BE
continue consuming/writing input instead of cancelling promptly, but the
eventual commit still uses that transaction ID and fails with `transaction
[...] not found` (`ANALYSIS_ERROR`, which the new helper does not retry).
Graceful FE shutdown does not hand off these BE-owned transactions. Couple
continuation to transaction-state preservation/handoff, or restrict the
exemption to transactions that can survive the master loss; cover a real
begin-election-commit sequence. This case excludes already-precommitted and
centrally managed cloud transactions.
##########
be/src/util/dns_cache.cpp:
##########
@@ -249,8 +278,59 @@ bool DNSCache::_erase_if_still_failing(const std::string&
hostname, uint32_t thr
return true;
}
-Status DNSCache::_update(const std::string& hostname, FailureState* out_state,
- std::string* out_ip) {
+Status DNSCache::_update(const std::string& hostname, FailureState* out_state,
std::string* out_ip,
+ bool reuse_cached) {
+ 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) {
Review Comment:
[P3] Release lookup ownership before checking for an idle mutex
With two concurrent lookups for one hostname, A can unlock the hostname
mutex, observe map+A+B references here, skip erasure, then pause before its
local `refresh_mutex` is destroyed. B can finish and also observe three
references before A resumes. Both locals then disappear, leaving the map as
sole owner with no cleanup left to run. The new lock map consequently retains
an idle key, mutex and control block until another lookup or cache destruction,
including for failed/evicted hostnames that are never used again. Release this
call's reference under the cache mutex before deciding whether the map is the
sole remaining owner, or track active users under that mutex. The existing
concurrent cold-miss test does not check this final-exit 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]
