Gabriel39 commented on code in PR #67053:
URL: https://github.com/apache/doris/pull/67053#discussion_r3840835152
##########
be/src/common/daemon.cpp:
##########
@@ -454,9 +454,13 @@ void Daemon::calculate_metrics_thread() {
}
void Daemon::report_runtime_query_statistics_thread() {
- while (!_stop_background_threads_latch.wait_for(
-
std::chrono::milliseconds(config::report_query_statistics_interval_ms))) {
-
ExecEnv::GetInstance()->runtime_query_statistics_mgr()->report_runtime_query_statistics();
+ auto* statistics_mgr =
ExecEnv::GetInstance()->runtime_query_statistics_mgr();
+ while (_stop_background_threads_latch.count() != 0) {
+ statistics_mgr->wait_for_runtime_query_statistics_report(
Review Comment:
Resolved by removing the completion-triggered global reporter entirely. The
periodic full reconciliation keeps its original fixed cadence; each completed
LOAD now adds one small statistics payload to its existing final fragment
report.
##########
be/src/common/daemon.cpp:
##########
@@ -454,9 +454,13 @@ void Daemon::calculate_metrics_thread() {
}
void Daemon::report_runtime_query_statistics_thread() {
- while (!_stop_background_threads_latch.wait_for(
-
std::chrono::milliseconds(config::report_query_statistics_interval_ms))) {
-
ExecEnv::GetInstance()->runtime_query_statistics_mgr()->report_runtime_query_statistics();
+ auto* statistics_mgr =
ExecEnv::GetInstance()->runtime_query_statistics_mgr();
+ while (_stop_background_threads_latch.count() != 0) {
+ statistics_mgr->wait_for_runtime_query_statistics_report(
+
std::chrono::milliseconds(config::report_query_statistics_interval_ms));
+ if (_stop_background_threads_latch.count() != 0) {
+ statistics_mgr->report_runtime_query_statistics();
Review Comment:
Resolved. Final statistics now piggyback on the existing per-fragment report
to that query coordinator, so there is no new serial cross-FE RPC loop. Each
final callback is scheduled independently, and FE processes the statistics in
the same RPC before it processes query completion and queues the audit event.
##########
be/src/runtime/query_context.cpp:
##########
@@ -248,6 +248,13 @@ QueryContext::~QueryContext() {
PrettyPrinter::print_bytes(query_mem_tracker()->peak_consumption()));
}
_resource_ctx->task_controller()->finish();
Review Comment:
Resolved by using the atomic finish timestamp itself as the completion
state. A release CAS publishes the nonzero timestamp and acquire reads
determine both completion and age, so the finished-with-zero-timestamp state no
longer exists. The last-fragment test covers the published invariant.
##########
be/src/runtime/query_context.cpp:
##########
@@ -248,6 +248,13 @@ QueryContext::~QueryContext() {
PrettyPrinter::print_bytes(query_mem_tracker()->peak_consumption()));
}
_resource_ctx->task_controller()->finish();
+#ifndef BE_TEST
+ if (_resource_ctx->task_controller()->query_type() == TQueryType::LOAD) {
Review Comment:
Resolved for the affected external-table DML path. Reporting no longer
depends on QueryContext destruction. It occurs at the explicit final
fragment-close edge after pipeline tasks and sink writers have closed,
including the recursive-CTE FINAL_CLOSE path, and only the last local LOAD
fragment attaches the ResourceContext snapshot.
##########
be/src/runtime/query_context.cpp:
##########
@@ -248,6 +248,13 @@ QueryContext::~QueryContext() {
PrettyPrinter::print_bytes(query_mem_tracker()->peak_consumption()));
}
_resource_ctx->task_controller()->finish();
+#ifndef BE_TEST
+ if (_resource_ctx->task_controller()->query_type() == TQueryType::LOAD) {
+ // Final DML counters must reach FE before its audit timeout. Periodic
reporting remains
+ // the retry path.
+
_exec_env->runtime_query_statistics_mgr()->trigger_runtime_query_statistics_report();
Review Comment:
Resolved. The asynchronous final request retains the exact shared
ResourceContext generation and reports from that object without a map lookup.
Successful direct and periodic cleanup now erase only when the current map
value has the same identity. A same-query-ID replacement test verifies that
cleanup cannot remove the newer generation.
##########
be/test/runtime/runtime_query_statistics_mgr_test.cpp:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "runtime/runtime_query_statistics_mgr.h"
+
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <future>
+
+namespace doris {
+
+TEST(RuntimeQueryStatisticsMgrTest,
QueryFinishTriggerWakesReporterAndCoalesces) {
Review Comment:
Resolved by replacing the notification-bit test and removing the
BE_TEST-excluded trigger path. The new tests execute the production payload
builder used by PipelineFragmentContext, the last-fragment completion gate,
identity-safe same-ID cleanup, and the existing external-file final-report
lifecycle.
--
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]
