refactor(query): reduce short-query execution overhead#20165
Conversation
|
https://github.com/dbsid/hbx-web3 is not accessable. |
🤖 CI Job Analysis (Retry 1)
📊 Summary
❌ NO RETRY NEEDEDAll failures appear to be code/test issues requiring manual fixes. 🔍 Job Details
🤖 AboutAutomated analysis using job annotations to distinguish infrastructure issues (auto-retried) from code/test issues (manual fixes needed). |
|
|
||
| // log the query event in the system log | ||
| info!("query: {} becomes {:?}", event.query_id, event.log_type); | ||
| if log::log_enabled!(target: "databend::log::query", log::Level::Info) { |
There was a problem hiding this comment.
log_enabled! does check dispatch settings, but only asks whether any dispatch accepts the metadata. So if one broad dispatch, such as QueryLogCollector, accepts it, log_enabled! returns true even when the specific history/file dispatch rejects it.
So, Unfortunately, in our system, this check almost always returns true.
You can test this by adding dbg! here, and set the config to
[log.query]
on = false
and will get
[src/query/service/src/interpreters/common/query_log.rs:131:9] log::log_enabled!(target: "databend::log::query", log::Level::Info) = true
[src/query/service/src/interpreters/common/query_log.rs:132:9] log::log_enabled!(target: "databend::log::query", log::Level::Debug) = true
[src/query/service/src/interpreters/common/query_log.rs:133:9] log::log_enabled!(target: "databend::log::query", log::Level::Trace) = true
There was a problem hiding this comment.
Fixed in 0475969. I removed the ineffective target-specific log_enabled! checks and gate query-log serialization/writes with GlobalConfig::instance().log.query.on before constructing the payload. Profile serialization is separately gated by log.profile.on; normal query execution and logging-on behavior remain unchanged.
There was a problem hiding this comment.
Follow-up eb9c16d925 keeps the fast path without relying on log_enabled!: query/profile/access payloads are enabled from the explicit file sink, the matching history-table sink, or an active per-thread capture. This also fixes history-only configurations that 0475969b07 did not cover. Focused history/capture tests, full single-threaded databend-query lib tests (198/198), and clippy with -D warnings pass.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Extract the short-query execution changes from #20144 into an independently reviewable PR.
This PR reduces fixed per-statement overhead in the MySQL, session, interpreter, logging, privilege-check, and pipeline executor paths. It does not change storage reads, lazy RowFetch, ordered top-k, or the physical sort implementation.
It adds no planner cache or query-result cache. Benchmarks explicitly use the same pre-existing mainline planner cache setting for baseline and PR (
enable_planner_cache=1) and disable query-result caching (enable_query_result_cache=0).Implementation
QueryPipelineExecutor::execute()as one pipeline worker, avoiding one additional OS thread spawn/join. BothPipelinePullingExecutorandPipelineCompleteExecutorinvoke it from their existing dedicated executor threads.The benchmark plans remain
Sort(Single)plans. None containsSort(PresortedMerge).Benchmark
Driver: https://github.com/dbsid/hbx-web3
Environment:
i4i.4xlarge, 16 vCPU Intel Xeon Platinum 8375C, single query node, S3 inus-west-2;RUSTFLAGS="-C target-cpu=native";max_threads=2,max_storage_io_requests=16, distributed pruning off;2026071305;lazy_read_threshold=1000and0.Binaries:
5876b2de8e: SHA256ce15f1261baa0103011abeab12c3e06fc4730deef656b12582affc8b0fa7d4ab;7524fce781: SHA25694d1df99896f11444650a5edd49ec73454d0305188e2228d5868fbde74b57407.The PR removes 7.05-8.21 ms with default lazy reads and 8.07-8.90 ms with lazy reads disabled. The no-lazy mean improvement is 17.7%-26.8%; the default-lazy mean improvement is 5.2%-6.3%. The similar absolute reduction across both modes is consistent with fixed execution overhead rather than a storage-plan change.
Correctness
lazy_read_threshold=0and1000: Q1 200/200, Q2 200/200, Flex 200/200; zero mismatches and zero errors.PresortedMerge.Validation
cargo fmt --all -- --checkgit diff --checkcargo clippy -p databend-query --lib --tests -- -D warningscargo test -p databend-query --lib -- --test-threads=1(198 passed)databend-querybuild withRUSTFLAGS="-C target-cpu=native"Tests
Type of change
This change is
Native protocol validation
The original benchmark table above used the MySQL driver. The same workload was also tested through Databend's non-MySQL HTTP protocol using
lake-go(lake://...:8000/default, HTTP/v1/query). This validates the native protocol path with the same 5 concurrent workers, fixed random seed2026071305, and 3,000 measured requests per query.The native HTTP A/B (the native path is unchanged by the follow-up safety commit
40c4cfcaab) produced the following round-2 results:All 36,000 native A/B requests succeeded with zero errors. On the final
40c4cfcaabbinary, a fresh no-lazy round also completed 3,000/3,000 requests for each of Q1, Q2, and Flex with means of 26.84, 40.20, and 40.35 ms respectively. Full ordered-result comparison on the final binary returned 100/100 matches for Q1, Q2, and Flex, with zero mismatches and zero errors.The benchmarked safety-follow-up release binary was built on the EC2 host with
RUSTFLAGS="-C target-cpu=native":40c4cfcaab47cc907b97a2c5ce374c57df0a6dd5;a754c99d3c3cc0a775d6c8fc34e29d3801a3ddc45eb49ec6884f0ce56f663be6.Applicability and large queries
The native HTTP and FlightSQL handlers both use
InterpreterFactory -> Interpreter::execute -> PipelinePullingExecutor/PipelineCompleteExecutor -> QueryPipelineExecutor, so the shared executor, session-settings, disabled-log, privilege, and query-id fast paths are protocol-independent. The shared MySQL runtime change inmysql_interactive_worker.rsis MySQL-only.The benchmark plans remain
Sort(Single); no result depends onSort(PresortedMerge), ordered top-k, lazy RowFetch, or storage block pruning. The pulling path keeps the same total worker count (threads_num - 1spawned workers plus the caller worker). Complete pipelines retain their dedicated executor thread, but that thread can itself serve as one pipeline worker. Thus the change removes fixed per-statement overhead, but does not reduce scan rows, scanned blocks, or analytical-query parallelism. Empty/no-source pipelines and callers that already run inside a Tokio runtime are not eligible for the inline worker shortcut.A controlled native HTTP A/B on the final workload used the following broad queries:
SELECT count(), sum(number), avg(number) FROM numbers(100000000);LIMIT 1000;All large-query samples completed without errors. These results support applicability to large queries as no-regression/neutral, not as a claim of material scan or execution-speedup.
Runtime safety follow-up
CI exposed that unconditionally running an inline worker can panic when a generic caller invokes
QueryPipelineExecutor::execute()inside a Tokio runtime (SyncSenderSink::blocking_send).40c4cfcaabkeeps the inline shortcut only on a non-Tokio thread and falls back to the original all-worker spawn behavior inside a runtime. Regression coverage now exercises both caller modes and verifies that the pipeline finish hook runs exactly once.Configured logging follow-up
Deep review found that checking only
log.query.on,log.profile.on, or the tracing target could suppress payload construction when a history-table sink or per-thread dynamic capture was the actual consumer.eb9c16d925checks the configured query/profile/access sink explicitly and keeps the disabled-log fast path only when no consumer needs the event. It also adds focused tests for history-only configuration, dynamic capture, executor caller modes, and query-context settings snapshots.The latest review-fix release binary was built on the EC2 host with
RUSTFLAGS="-C target-cpu=native":eb9c16d92556a6ab09c09e0e35950ba6b9d6e323;d18d8b82eb5f37e9955951b2f8d1ed1a49a63d70faa5c4e324b956e993bf15c8.Native correctness follow-up
After the service recovered, the final
40c4cfcaabbinary was rechecked throughlake-goover native HTTP with the same deterministic parameter generator:Q1, Q2, and Flex each returned
500/500ordered-result matches, with zero mismatches and zero errors (1,500/1,500total). The JSON artifacts arefinal/native-correctness-i500-seed2026071305.jsonandfinal/native-correctness-flex-i500-seed2026071305.jsonunder the benchmark result directory.go test ./...for the driver also passed.Query-shape qualification
The optimization is protocol-independent for correctness, but its measurable benefit is query-shape dependent:
SELECTqueries that use a pulling pipeline reusePipelinePullingExecutor's dedicated thread as one worker and receive the shared fixed-overhead reductions.INSERT,UPDATE,DELETE, CTAS, and other complete pipelines still useCompleteExecutor's dedicated thread. On a non-Tokio thread it also serves as one pipeline worker, so complete pipelines avoid one additional worker spawn while preserving their dedicated-executor boundary.blocking_sendpanics.Therefore “effective for all queries” means the change is safe on the common public execution paths, not that every query shape gets the same speedup. For large search or analytical queries it is suitable as a no-regression refactoring, but the expected absolute gain is small relative to scan, join, aggregation, and sort time. It should not be presented as a block-pruning or analytical-query acceleration.