github-actions[bot] commented on code in PR #67006:
URL: https://github.com/apache/doris/pull/67006#discussion_r3824293588
##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -322,6 +325,11 @@ FragmentMgr::FragmentMgr(ExecEnv* exec_env)
: _exec_env(exec_env), _stop_background_threads_latch(1) {
_entity =
DorisMetrics::instance()->metric_registry()->register_entity("FragmentMgr");
INT_UGAUGE_METRIC_REGISTER(_entity, timeout_canceled_fragment_count);
+ REGISTER_ENTITY_HOOK_METRIC(_entity, this, query_ctx_delay_delete_count,
+ [this]() { return
_query_ctx_map_delay_delete.num_items(); });
+ REGISTER_ENTITY_HOOK_METRIC(_entity, this,
query_ctx_delay_delete_oldest_age_seconds, [this]() {
Review Comment:
[P2] Avoid building the full HTTP snapshot for this scalar gauge
This hook needs only the maximum age, but
`get_query_ctx_map_delay_delete_stats()` allocates/pushes one record per
retained query while `ConcurrentContextMap::apply()` holds each shard's unique
lock. Metric hooks run every 15 seconds while `MetricRegistry` also holds its
registry/entity locks, so a retention buildup makes every scrape do O(N)
allocation/copying and block inserts/erases even though the vector is
discarded. Please use a scalar-only read traversal for the gauge and reserve
the full snapshot for the on-demand HTTP dump.
##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -1189,10 +1237,15 @@ Status FragmentMgr::merge_filter(const
PMergeFilterRequest* request,
query_id.__set_lo(queryid.lo);
if (auto q_ctx = get_query_ctx(query_id)) {
SCOPED_ATTACH_TASK(q_ctx.get());
- if (!q_ctx->get_merge_controller_handler()) {
+ auto handler = q_ctx->get_merge_controller_handler();
+ if (!handler) {
return Status::InternalError("Merge filter failed: Merge
controller handler is null");
}
- return q_ctx->get_merge_controller_handler()->merge(q_ctx, request,
attach_data);
+ auto status = handler->merge(q_ctx, request, attach_data);
+ if (handler->all_filters_published()) {
Review Comment:
[P1] Preserve delayed ownership across recursive-filter generations
`all_filters_published()` is not query-terminal for recursive CTEs. After
round N makes every `done` bit true, this erases the only post-EOS strong
reference. The next `reset_global_rf()` explicitly sets those contexts back to
`done=false` and advances their stage, but neither reset path re-inserts the
`QueryContext`. A later/final-round producer posts `merge_filter`
asynchronously, so its PFC/root can reach EOS before that RPC is serviced; at
that point `_query_ctx_map` is only weak and the merge returns EOF instead of
publishing the filter. Please make retention generation-aware (and serialize it
with reset), or erase only after an explicit final generation, and add a
multi-round delayed-merge test.
--
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]
