Skip to content

Commit d068568

Browse files
feat: introduce new setting enable_last_snapshot_location_hint (#16226)
* feat: introduce new setting `enable_last_snapshot_location_hint` - Which indicates if `last_snapshot_location_hint` should be kept or not. - It is enabled by default * refactor: let `write_last_snapshot_hint` handles the setting * chore: license info * logic test for setting enable_last_snapshot_location_hint * chore: fix typo in test script --------- Co-authored-by: Sky Fan <3374614481@qq.com>
1 parent d24996b commit d068568

File tree

10 files changed

+95
-11
lines changed

10 files changed

+95
-11
lines changed

โ€Žsrc/query/service/src/interpreters/interpreter_table_add_column.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ impl Interpreter for AddTableColumnInterpreter {
112112
};
113113
new_table_meta.add_column(&field, &self.plan.comment, index)?;
114114

115-
let _ = generate_new_snapshot(tbl.as_ref(), &mut new_table_meta).await?;
115+
let _ = generate_new_snapshot(self.ctx.as_ref(), tbl.as_ref(), &mut new_table_meta).await?;
116116
let table_id = table_info.ident.table_id;
117117
let table_version = table_info.ident.seq;
118118

@@ -162,6 +162,7 @@ impl Interpreter for AddTableColumnInterpreter {
162162
}
163163

164164
pub(crate) async fn generate_new_snapshot(
165+
ctx: &QueryContext,
165166
table: &dyn Table,
166167
new_table_meta: &mut TableMeta,
167168
) -> Result<()> {
@@ -188,16 +189,16 @@ pub(crate) async fn generate_new_snapshot(
188189

189190
// write down hint
190191
FuseTable::write_last_snapshot_hint(
192+
ctx,
191193
fuse_table.get_operator_ref(),
192194
fuse_table.meta_location_generator(),
193-
new_snapshot_location.clone(),
195+
&new_snapshot_location,
194196
)
195197
.await;
196198

197-
new_table_meta.options.insert(
198-
OPT_KEY_SNAPSHOT_LOCATION.to_owned(),
199-
new_snapshot_location.clone(),
200-
);
199+
new_table_meta
200+
.options
201+
.insert(OPT_KEY_SNAPSHOT_LOCATION.to_owned(), new_snapshot_location);
201202
} else {
202203
info!("Snapshot not found, no need to generate new snapshot");
203204
}

โ€Žsrc/query/service/src/interpreters/interpreter_table_drop_column.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl Interpreter for DropTableColumnInterpreter {
130130
let table_id = table_info.ident.table_id;
131131
let table_version = table_info.ident.seq;
132132

133-
generate_new_snapshot(table.as_ref(), &mut new_table_meta).await?;
133+
generate_new_snapshot(self.ctx.as_ref(), table.as_ref(), &mut new_table_meta).await?;
134134

135135
let req = UpdateTableMetaReq {
136136
table_id,

โ€Žsrc/query/settings/src/settings_default.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,12 @@ impl DefaultSettings {
813813
mode: SettingMode::Both,
814814
range: Some(SettingRange::Numeric(0..=1)),
815815
}),
816+
("enable_last_snapshot_location_hint", DefaultSettingValue {
817+
value: UserSettingValue::UInt64(1),
818+
desc: "Enables writing last_snapshot_location_hint object",
819+
mode: SettingMode::Both,
820+
range: Some(SettingRange::Numeric(0..=1)),
821+
}),
816822
("format_null_as_str", DefaultSettingValue {
817823
value: UserSettingValue::UInt64(1),
818824
desc: "Format NULL as str in query api response",

โ€Žsrc/query/settings/src/settings_getter_setter.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,4 +677,8 @@ impl Settings {
677677
pub fn get_format_null_as_str(&self) -> Result<bool> {
678678
Ok(self.try_get_u64("format_null_as_str")? == 1)
679679
}
680+
681+
pub fn get_enable_last_snapshot_location_hint(&self) -> Result<bool> {
682+
Ok(self.try_get_u64("enable_last_snapshot_location_hint")? == 1)
683+
}
680684
}

โ€Žsrc/query/storages/fuse/src/operations/commit.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ impl FuseTable {
149149
let catalog = ctx.get_catalog(table_info.catalog()).await?;
150150
// 2. update table meta
151151
let res = Self::update_table_meta(
152+
ctx,
152153
catalog,
153154
table_info,
154155
location_generator,
@@ -207,6 +208,7 @@ impl FuseTable {
207208
#[allow(clippy::too_many_arguments)]
208209
#[async_backtrace::framed]
209210
pub async fn update_table_meta(
211+
ctx: &dyn TableContext,
210212
catalog: Arc<dyn Catalog>,
211213
table_info: &TableInfo,
212214
location_generator: &TableMetaLocationGenerator,
@@ -242,18 +244,27 @@ impl FuseTable {
242244

243245
// update_table_meta succeed, populate the snapshot cache item and try keeping a hit file of last snapshot
244246
TableSnapshot::cache().put(snapshot_location.clone(), Arc::new(snapshot));
245-
Self::write_last_snapshot_hint(operator, location_generator, snapshot_location).await;
247+
Self::write_last_snapshot_hint(ctx, operator, location_generator, &snapshot_location).await;
246248

247249
Ok(())
248250
}
249251

250252
// Left a hint file which indicates the location of the latest snapshot
251253
#[async_backtrace::framed]
252254
pub async fn write_last_snapshot_hint(
255+
ctx: &dyn TableContext,
253256
operator: &Operator,
254257
location_generator: &TableMetaLocationGenerator,
255-
last_snapshot_path: String,
258+
last_snapshot_path: &str,
256259
) {
260+
if let Ok(false) = ctx.get_settings().get_enable_last_snapshot_location_hint() {
261+
info!(
262+
"Write last_snapshot_location_hint disabled. Snapshot {}",
263+
last_snapshot_path
264+
);
265+
return;
266+
}
267+
257268
// Just try our best to write down the hint file of last snapshot
258269
// - will retry in the case of temporary failure
259270
// but

โ€Žsrc/query/storages/fuse/src/operations/common/processors/sink_commit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ where F: SnapshotGenerator + Send + 'static
365365

366366
let catalog = self.ctx.get_catalog(table_info.catalog()).await?;
367367
match FuseTable::update_table_meta(
368+
self.ctx.as_ref(),
368369
catalog.clone(),
369370
&table_info,
370371
&self.location_gen,

โ€Žsrc/query/storages/fuse/src/operations/revert.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,15 @@ impl FuseTable {
6363
// 4. let's roll
6464
let reply = catalog.update_single_table_meta(req, table_info).await;
6565
if reply.is_ok() {
66-
// try keep the snapshot hit
66+
// try keeping the snapshot hit
6767
let snapshot_location = table_reverting_to.snapshot_loc().await?.ok_or_else(|| {
6868
ErrorCode::Internal("internal error, fuse table which navigated to given point has no snapshot location")
6969
})?;
7070
Self::write_last_snapshot_hint(
71+
ctx.as_ref(),
7172
&table_reverting_to.operator,
7273
&table_reverting_to.meta_location_generator,
73-
snapshot_location,
74+
&snapshot_location,
7475
)
7576
.await;
7677
};
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
2+
3+
statement ok
4+
create or replace database snapshot_hint;
5+
6+
statement ok
7+
use snapshot_hint;
8+
9+
# Disable keeping snapshot location hint
10+
statement ok
11+
set enable_last_snapshot_location_hint = 0;
12+
13+
statement ok
14+
create or replace table t (c int) 'fs:///tmp/snapshot_hint_disabled/' as select * from numbers(1);
15+
16+
# create a stage that points to the same location of table `t`
17+
statement ok
18+
create or replace stage hint_stage url='fs:///tmp/snapshot_hint_disabled/';
19+
20+
21+
# Table created with CTAS, should have no snapshot hint file
22+
query T
23+
select name from list_stage(location => '@hint_stage') where name like '%last_snapshot_location_hint%';
24+
----
25+
26+
statement ok
27+
insert into t values(2);
28+
29+
# After insertion, there should be no snapshot hint file
30+
query T
31+
select name from list_stage(location => '@hint_stage') where name like '%last_snapshot_location_hint%';
32+
----

โ€Žtests/sqllogictests/suites/ee/03_ee_vacuum/03_0000_vacuum_ctas.test

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
## Copyright 2023 Databend Cloud
2+
##
3+
## Licensed under the Elastic 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+
## https://www.elastic.co/licensing/elastic-license
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+
115
# test orphan data created by failed CTAS could be vacuumed
216
statement ok
317
drop database if exists ctas_test;

โ€Žtests/sqllogictests/suites/ee/03_ee_vacuum/03_0000_vacuum_temporary_files.test

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
## Copyright 2023 Databend Cloud
2+
##
3+
## Licensed under the Elastic 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+
## https://www.elastic.co/licensing/elastic-license
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+
115
onlyif mysql
216
statement ok
317
set max_threads = 8;

0 commit comments

Comments
ย (0)