Skip to content

Commit 8933e96

Browse files
authored
refactor: remove redundant sql from result cache meta (#19736)
* feat: limit result cache by sql length * refactor: store result cache sql hash in meta * refactor: remove redundant sql from result cache meta * refactor: show query id in result cache explain * refactor: keep result cache explain stable * refactor: log query id for result cache access * fix: resolve result cache ci failures * fix(storage): reorder basic Cargo deps * test: update result cache explain sqllogic outputs * test: remove sql from result cache explain outputs
1 parent 1b12f52 commit 8933e96

10 files changed

Lines changed: 13 additions & 18 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/query/service/src/interpreters/interpreter_explain.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,6 @@ impl ExplainInterpreter {
364364
if let Some(v) = cache_reader.check_cache().await? {
365365
// Construct a format tree for result cache reading
366366
let children = vec![
367-
FormatTreeNode::new(format!("SQL: {}", v.sql)),
368367
FormatTreeNode::new(format!("Number of rows: {}", v.num_rows)),
369368
FormatTreeNode::new(format!("Result size: {}", v.result_size)),
370369
];

src/query/service/tests/it/storages/testdata/columns_table.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,6 @@ DB.Table: 'system'.'columns', Table: columns-table_id:1, ver:0, Engine: SystemCo
394394
| 'snapshot_location' | 'system' | 'streams' | 'Nullable(String)' | 'VARCHAR' | '' | '' | 'YES' | '' |
395395
| 'source' | 'system' | 'dictionaries' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' |
396396
| 'source_column' | 'system' | 'virtual_columns' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' |
397-
| 'sql' | 'system' | 'query_cache' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' |
398397
| 'sql_path' | 'information_schema' | 'schemata' | 'NULL' | 'NULL' | '' | '' | 'NO' | '' |
399398
| 'stack' | 'system' | 'backtrace' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' |
400399
| 'stage_type' | 'system' | 'stages' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' |

src/query/storages/basic/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ databend-common-storages-parquet = { workspace = true }
2222
databend-meta-client = { workspace = true }
2323
databend-storages-common-blocks = { workspace = true }
2424
databend-storages-common-table-meta = { workspace = true }
25+
log = { workspace = true }
2526
opendal = { workspace = true }
2627
parking_lot = { workspace = true }
2728
parquet = { workspace = true }

src/query/storages/basic/src/result_cache/common.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ pub(crate) fn gen_result_cache_dir(key: &str) -> String {
3939

4040
#[derive(serde::Serialize, serde::Deserialize)]
4141
pub struct ResultCacheValue {
42-
/// The original query SQL.
43-
pub sql: String,
4442
/// Associated query id
4543
pub query_id: String,
4644
/// The query time.

src/query/storages/basic/src/result_cache/read/reader.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use databend_common_expression::DataBlock;
2020
use databend_common_expression::DataSchema;
2121
use databend_common_meta_store::MetaStore;
2222
use databend_common_storage::DataOperator;
23+
use log::info;
2324
use opendal::Operator;
2425
use parquet::arrow::arrow_reader::ParquetRecordBatchReader;
2526

@@ -68,6 +69,10 @@ impl ResultCacheReader {
6869
pub async fn check_cache(&self) -> Result<Option<ResultCacheValue>> {
6970
if let Some(v) = self.meta_mgr.get(self.meta_key.clone()).await? {
7071
if self.tolerate_inconsistent || v.partitions_shas == self.partitions_shas {
72+
info!(
73+
"Query result cache hit: query_id={}, meta_key={}",
74+
v.query_id, self.meta_key
75+
);
7176
return Ok(Some(v));
7277
}
7378
}

src/query/storages/basic/src/result_cache/write/sink.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use databend_common_pipeline::sinks::AsyncMpscSink;
2727
use databend_common_pipeline::sinks::AsyncMpscSinker;
2828
use databend_common_storage::DataOperator;
2929
use databend_meta_client::types::MatchSeq;
30+
use log::info;
3031
use tokio::time::Instant;
3132

3233
use super::writer::ResultCacheWriter;
@@ -37,7 +38,6 @@ use crate::result_cache::meta_manager::ResultCacheMetaManager;
3738

3839
pub struct WriteResultCacheSink {
3940
ctx: Arc<dyn TableContext>,
40-
sql: String,
4141
partitions_shas: Vec<String>,
4242

4343
meta_mgr: ResultCacheMetaManager,
@@ -109,7 +109,6 @@ impl AsyncMpscSink for WriteResultCacheSink {
109109
let ttl_interval = Duration::from_secs(ttl_sec);
110110

111111
let value = ResultCacheValue {
112-
sql: self.sql.clone(),
113112
query_id: self.ctx.get_id(),
114113
query_time: now,
115114
ttl: ttl_sec,
@@ -122,6 +121,11 @@ impl AsyncMpscSink for WriteResultCacheSink {
122121
self.meta_mgr
123122
.set(self.meta_key.clone(), value, MatchSeq::GE(0), ttl_interval)
124123
.await?;
124+
info!(
125+
"Query result cache write: query_id={}, meta_key={}",
126+
self.ctx.get_id(),
127+
self.meta_key
128+
);
125129
self.ctx
126130
.set_query_id_result_cache(self.ctx.get_id(), self.meta_key.clone());
127131
Ok(())
@@ -141,7 +145,6 @@ impl WriteResultCacheSink {
141145
let min_execute_secs = settings.get_query_result_cache_min_execute_secs()?;
142146
let ttl = settings.get_query_result_cache_ttl_secs()?;
143147
let tenant = ctx.get_tenant();
144-
let sql = ctx.get_query_str();
145148
let partitions_shas = ctx.get_partitions_shas();
146149

147150
let meta_key = gen_result_cache_meta_key(tenant.tenant_name(), key);
@@ -155,7 +158,6 @@ impl WriteResultCacheSink {
155158
inputs,
156159
WriteResultCacheSink {
157160
ctx,
158-
sql,
159161
partitions_shas,
160162
meta_mgr: ResultCacheMetaManager::create(kv_store, ttl),
161163
meta_key,

src/query/storages/system/src/query_cache_table.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ impl AsyncSystemTable for QueryCacheTable {
6363

6464
let cached_values = result_cache_mgr.list(prefix.as_str()).await?;
6565

66-
let mut sql_vec: Vec<&str> = Vec::with_capacity(cached_values.len());
6766
let mut query_id_vec: Vec<&str> = Vec::with_capacity(cached_values.len());
6867
let mut result_size_vec = Vec::with_capacity(cached_values.len());
6968
let mut num_rows_vec = Vec::with_capacity(cached_values.len());
@@ -73,7 +72,6 @@ impl AsyncSystemTable for QueryCacheTable {
7372
let mut active_result_scan: Vec<bool> = Vec::with_capacity(cached_values.len());
7473

7574
cached_values.iter().for_each(|x| {
76-
sql_vec.push(x.sql.as_str());
7775
query_id_vec.push(x.query_id.as_str());
7876
result_size_vec.push(x.result_size as u64);
7977
num_rows_vec.push(x.num_rows as u64);
@@ -103,7 +101,6 @@ impl AsyncSystemTable for QueryCacheTable {
103101
.collect();
104102

105103
Ok(DataBlock::new_from_columns(vec![
106-
StringType::from_data(sql_vec),
107104
StringType::from_data(query_id_vec),
108105
UInt64Type::from_data(result_size_vec),
109106
UInt64Type::from_data(num_rows_vec),
@@ -128,7 +125,6 @@ impl AsyncSystemTable for QueryCacheTable {
128125
impl QueryCacheTable {
129126
pub fn create(table_id: u64) -> Arc<dyn Table> {
130127
let schema = TableSchemaRefExt::create(vec![
131-
TableField::new("sql", TableDataType::String),
132128
TableField::new("query_id", TableDataType::String),
133129
TableField::new("result_size", TableDataType::Number(NumberDataType::UInt64)),
134130
TableField::new("num_rows", TableDataType::Number(NumberDataType::UInt64)),

tests/sqllogictests/suites/base/20+_others/20_0013_query_result_cache.test

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,13 @@ query T
7979
EXPLAIN SELECT * FROM t1 ORDER BY a;
8080
----
8181
ReadQueryResultCache
82-
├── SQL: SELECT * FROM t1 ORDER BY a
8382
├── Number of rows: 3
8483
└── Result size: 12
8584

8685
query T
8786
EXPLAIN SELECT * FROM t1, t2 ORDER BY a, b;
8887
----
8988
ReadQueryResultCache
90-
├── SQL: SELECT * FROM t1, t2 ORDER BY a, b
9189
├── Number of rows: 9
9290
└── Result size: 180
9391

@@ -320,7 +318,6 @@ query T
320318
EXPLAIN SELECT *, (SELECT MIN(b) FROM t_scalar_subquery) as min_val FROM t_scalar_subquery ORDER BY a;
321319
----
322320
ReadQueryResultCache
323-
├── SQL: SELECT *, (SELECT MIN(b) FROM t_scalar_subquery) AS min_val FROM t_scalar_subquery ORDER BY a
324321
├── Number of rows: 3
325322
└── Result size: 39
326323

tests/sqllogictests/suites/ee/05_ee_ddl/05_0011_row_policy_result_cache.test

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ query T
7272
EXPLAIN SELECT * FROM rap_cache_test ORDER BY id;
7373
----
7474
ReadQueryResultCache
75-
├── SQL: SELECT * FROM rap_cache_test ORDER BY id
7675
├── Number of rows: 2
7776
└── Result size: 119
7877

@@ -126,7 +125,6 @@ query T
126125
EXPLAIN SELECT * FROM rap_cache_test ORDER BY id;
127126
----
128127
ReadQueryResultCache
129-
├── SQL: SELECT * FROM rap_cache_test ORDER BY id
130128
├── Number of rows: 3
131129
└── Result size: 177
132130

@@ -142,7 +140,6 @@ query T
142140
EXPLAIN SELECT * FROM rap_cache_test ORDER BY id;
143141
----
144142
ReadQueryResultCache
145-
├── SQL: SELECT * FROM rap_cache_test ORDER BY id
146143
├── Number of rows: 2
147144
└── Result size: 119
148145

0 commit comments

Comments
 (0)