Skip to content

Commit 9b5c450

Browse files
authored
fix: some queries print query log and profile twice (#19455)
* refactor(logging): replace on_execution_finished with QueryFinishHooks fix(logging): prevent duplicate query logs by fixing root causes fix(logging): deduplicate query logs and improve log tests * fix test clean code * apply suggestions
1 parent 8a52d48 commit 9b5c450

12 files changed

Lines changed: 382 additions & 126 deletions

File tree

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::sync::Arc;
16+
17+
use databend_common_catalog::table_context::TableContext;
18+
use databend_common_exception::Result;
19+
use databend_common_pipeline::core::ExecutionInfo;
20+
21+
use crate::interpreters::common::log_query_finished;
22+
use crate::interpreters::hook::vacuum_hook::hook_clear_m_cte_temp_table;
23+
use crate::interpreters::hook::vacuum_hook::hook_disk_temp_dir;
24+
use crate::interpreters::hook::vacuum_hook::hook_vacuum_temp_files;
25+
use crate::sessions::QueryContext;
26+
27+
fn run_hooks(query_ctx: Arc<QueryContext>) -> Result<()> {
28+
hook_clear_m_cte_temp_table(&query_ctx)?;
29+
hook_vacuum_temp_files(&query_ctx)?;
30+
hook_disk_temp_dir(&query_ctx)
31+
}
32+
33+
/// Controls which post-execution actions are performed when a pipeline finishes.
34+
///
35+
/// Use [`QueryFinishHooks::top_level`] for normal user-facing queries,
36+
/// [`QueryFinishHooks::nested_with_hooks`] for internal sub-executions that may
37+
/// create temporary artifacts (e.g. EXPLAIN ANALYZE, EXPLAIN PERF inner pipelines),
38+
/// and [`QueryFinishHooks::nested`] for lightweight internal pipelines that don't
39+
/// need cleanup (e.g. recursive CTE inner pipeline).
40+
pub struct QueryFinishHooks {
41+
/// Collect pipeline execution profiles into the query context.
42+
pub collect_profiles: bool,
43+
/// Run post-query cleanup hooks (CTE temp tables, spill files, disk temp dirs).
44+
pub run_hooks: bool,
45+
/// Emit the query-finish log, metrics, and profile JSON.
46+
pub log_finished: bool,
47+
}
48+
49+
impl QueryFinishHooks {
50+
/// All three actions enabled. Use for top-level user queries.
51+
pub fn top_level() -> Self {
52+
Self {
53+
collect_profiles: true,
54+
run_hooks: true,
55+
log_finished: true,
56+
}
57+
}
58+
59+
/// Profiles only — no hooks, no logging. Use for nested/internal pipeline
60+
/// executions where the outer query owns the lifecycle (e.g. recursive CTE
61+
/// inner pipeline).
62+
pub fn nested() -> Self {
63+
Self {
64+
collect_profiles: true,
65+
run_hooks: false,
66+
log_finished: false,
67+
}
68+
}
69+
70+
/// Profiles and cleanup hooks, but no logging. Use for nested pipelines
71+
/// that may create temporary artifacts (spill files, CTE temp tables) and
72+
/// need cleanup even on failure, while the outer query owns the log
73+
/// lifecycle (e.g. EXPLAIN ANALYZE, EXPLAIN PERF inner pipelines).
74+
pub fn nested_with_hooks() -> Self {
75+
Self {
76+
collect_profiles: true,
77+
run_hooks: true,
78+
log_finished: false,
79+
}
80+
}
81+
82+
/// Convert into a closure suitable for [`Pipeline::set_on_finished`].
83+
pub fn into_callback(
84+
self,
85+
ctx: Arc<QueryContext>,
86+
) -> impl Fn(&ExecutionInfo) -> Result<()> + Send + Sync + 'static {
87+
move |info: &ExecutionInfo| {
88+
if self.collect_profiles {
89+
ctx.add_query_profiles(&info.profiling);
90+
}
91+
let hooks_res = if self.run_hooks {
92+
run_hooks(ctx.clone())
93+
} else {
94+
Ok(())
95+
};
96+
if self.log_finished {
97+
log_query_finished(&ctx, info.res.clone().err());
98+
}
99+
info.res.clone().and(hooks_res)
100+
}
101+
}
102+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::collections::BTreeMap;
16+
use std::sync::Arc;
17+
use std::time::SystemTime;
18+
19+
use databend_common_base::runtime::profile::ProfileDesc;
20+
use databend_common_base::runtime::profile::ProfileStatisticsName;
21+
use databend_common_base::runtime::profile::get_statistics_desc;
22+
use databend_common_catalog::table_context::TableContext;
23+
use databend_common_exception::ErrorCode;
24+
use databend_common_pipeline::core::PlanProfile;
25+
use log::error;
26+
use log::info;
27+
28+
use crate::interpreters::InterpreterMetrics;
29+
use crate::interpreters::InterpreterQueryLog;
30+
use crate::sessions::QueryContext;
31+
use crate::sessions::SessionManager;
32+
33+
pub fn log_query_start(ctx: &QueryContext) {
34+
InterpreterMetrics::record_query_start(ctx);
35+
let now = SystemTime::now();
36+
let session = ctx.get_current_session();
37+
let typ = session.get_type();
38+
if typ.is_user_session() {
39+
SessionManager::instance().status.write().query_start(now);
40+
}
41+
42+
if let Err(error) = InterpreterQueryLog::log_start(ctx, now, None) {
43+
error!("Failed to log query start: {:?}", error)
44+
}
45+
}
46+
47+
pub fn log_query_finished(ctx: &QueryContext, error: Option<ErrorCode>) {
48+
// metrics
49+
InterpreterMetrics::record_query_finished(ctx, error.clone());
50+
51+
let now = SystemTime::now();
52+
let session = ctx.get_current_session();
53+
54+
session.get_status().write().query_finish();
55+
let typ = session.get_type();
56+
if typ.is_user_session() {
57+
SessionManager::instance().status.write().query_finish(now);
58+
SessionManager::instance()
59+
.metrics_collector
60+
.track_finished_query(
61+
ctx.get_scan_progress_value(),
62+
ctx.get_write_progress_value(),
63+
ctx.get_join_spill_progress_value(),
64+
ctx.get_aggregate_spill_progress_value(),
65+
ctx.get_group_by_spill_progress_value(),
66+
ctx.get_window_partition_spill_progress_value(),
67+
);
68+
}
69+
70+
info!(memory:? = ctx.get_node_peek_memory_usage(); "total memory usage");
71+
72+
// databend::log::profile
73+
let query_profiles = ctx.get_query_profiles();
74+
let has_profiles = !query_profiles.is_empty();
75+
76+
if has_profiles {
77+
#[derive(serde::Serialize)]
78+
struct QueryProfiles {
79+
query_id: String,
80+
profiles: Vec<PlanProfile>,
81+
statistics_desc: Arc<BTreeMap<ProfileStatisticsName, ProfileDesc>>,
82+
}
83+
84+
match serde_json::to_string(&QueryProfiles {
85+
query_id: ctx.get_id(),
86+
profiles: query_profiles.clone(),
87+
statistics_desc: get_statistics_desc(),
88+
}) {
89+
Ok(profile_json) => {
90+
info!(target: "databend::log::profile", "{}", profile_json);
91+
}
92+
Err(err) => {
93+
error!("Failed to serialize query profiles: {:?}", err);
94+
}
95+
}
96+
}
97+
98+
// databend::log::query
99+
if let Err(error) = InterpreterQueryLog::log_finish(ctx, now, error, has_profiles) {
100+
error!("Failed to log query finish: {:?}", error)
101+
}
102+
}

src/query/service/src/interpreters/common/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
mod column;
16+
mod finish_hook;
1617
mod grant;
1718
mod metrics;
1819
mod notification;
@@ -22,10 +23,13 @@ mod table;
2223
mod task;
2324
mod util;
2425

26+
mod log;
2527
pub mod table_option_validation;
2628

2729
pub use column::*;
30+
pub use finish_hook::QueryFinishHooks;
2831
pub use grant::validate_grant_object_exists;
32+
pub use log::*;
2933
pub use notification::get_notification_client_config;
3034
pub use query_log::InterpreterQueryLog;
3135
pub use stream::dml_build_update_stream_req;

0 commit comments

Comments
 (0)