From 084b62dd60853d9e20d7a941948b746345a354aa Mon Sep 17 00:00:00 2001 From: dantengsky Date: Thu, 30 Mar 2023 20:43:59 +0800 Subject: [PATCH 1/3] do not upset copied-files set if both purge and force are enalbed --- .../src/catalogs/default/mutable_catalog.rs | 9 ++++++- .../src/interpreters/interpreter_copy.rs | 24 +++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/query/service/src/catalogs/default/mutable_catalog.rs b/src/query/service/src/catalogs/default/mutable_catalog.rs index f07989293467b..40ea5f5c1285e 100644 --- a/src/query/service/src/catalogs/default/mutable_catalog.rs +++ b/src/query/service/src/catalogs/default/mutable_catalog.rs @@ -287,7 +287,14 @@ impl Catalog for MutableCatalog { req: UpdateTableMetaReq, ) -> Result { match table_info.db_type.clone() { - DatabaseType::NormalDB => Ok(self.ctx.meta.update_table_meta(req).await?), + DatabaseType::NormalDB => { + info!( + "updating table meta. table desc: [{}], has copied files: [{}]", + table_info.desc, + req.copied_files.is_some() + ); + Ok(self.ctx.meta.update_table_meta(req).await?) + } DatabaseType::ShareDB(share_ident) => { let db = self .get_database(&share_ident.tenant, &share_ident.share_name) diff --git a/src/query/service/src/interpreters/interpreter_copy.rs b/src/query/service/src/interpreters/interpreter_copy.rs index e6ab2400f0087..4ddc9eb797268 100644 --- a/src/query/service/src/interpreters/interpreter_copy.rs +++ b/src/query/service/src/interpreters/interpreter_copy.rs @@ -260,6 +260,7 @@ impl CopyInterpreter { info!("end to list files: got {} files", num_all_files); let need_copy_file_infos = if force { + info!("force mode, ignore file filtering"); all_source_file_infos } else { // Status. @@ -429,13 +430,22 @@ impl CopyInterpreter { let table_id = to_table.get_id(); let expire_hours = ctx.get_settings().get_load_file_metadata_expire_hours()?; - let fail_if_duplicated = !force; - let upsert_copied_files_request = Self::upsert_copied_files_request( - table_id, - expire_hours, - copied_file_tree, - fail_if_duplicated, - ); + let upsert_copied_files_request = { + if stage_info.copy_options.purge && force { + // if `purge-after-copy` is enabled, and in `force` copy mode, + // we do not need to upsert copied files into meta server + info!("[purge-after-copy] and [force-copy] are both enabled, will not update copied-files set"); + None + } else { + let fail_if_duplicated = !force; + Self::upsert_copied_files_request( + table_id, + expire_hours, + copied_file_tree, + fail_if_duplicated, + ) + } + }; { let status = format!("begin commit, number of copied files:{}", num_copied_files); From 12d7b19b7ea70e2104d4a4ee3f779fc3ebbee772 Mon Sep 17 00:00:00 2001 From: dantengsky Date: Thu, 30 Mar 2023 20:55:04 +0800 Subject: [PATCH 2/3] adjust logging --- src/query/service/src/interpreters/interpreter_copy.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/query/service/src/interpreters/interpreter_copy.rs b/src/query/service/src/interpreters/interpreter_copy.rs index 4ddc9eb797268..bbadab89dca81 100644 --- a/src/query/service/src/interpreters/interpreter_copy.rs +++ b/src/query/service/src/interpreters/interpreter_copy.rs @@ -260,7 +260,10 @@ impl CopyInterpreter { info!("end to list files: got {} files", num_all_files); let need_copy_file_infos = if force { - info!("force mode, ignore file filtering"); + info!( + "force mode, ignore file filtering. ({}.{})", + database_name, table_name + ); all_source_file_infos } else { // Status. @@ -430,11 +433,12 @@ impl CopyInterpreter { let table_id = to_table.get_id(); let expire_hours = ctx.get_settings().get_load_file_metadata_expire_hours()?; + let table_info = to_table.get_table_info(); let upsert_copied_files_request = { if stage_info.copy_options.purge && force { // if `purge-after-copy` is enabled, and in `force` copy mode, // we do not need to upsert copied files into meta server - info!("[purge-after-copy] and [force-copy] are both enabled, will not update copied-files set"); + info!("[purge] and [force] are both enabled, will not update copied-files set. ({})", &table_info.desc); None } else { let fail_if_duplicated = !force; From a46df2f0d26beaa33ffb7304f09ba88676be3128 Mon Sep 17 00:00:00 2001 From: dantengsky Date: Thu, 30 Mar 2023 21:11:44 +0800 Subject: [PATCH 3/3] adjust logging message --- src/query/service/src/catalogs/default/mutable_catalog.rs | 2 +- src/query/service/src/interpreters/interpreter_copy.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/query/service/src/catalogs/default/mutable_catalog.rs b/src/query/service/src/catalogs/default/mutable_catalog.rs index 40ea5f5c1285e..829db0512de95 100644 --- a/src/query/service/src/catalogs/default/mutable_catalog.rs +++ b/src/query/service/src/catalogs/default/mutable_catalog.rs @@ -289,7 +289,7 @@ impl Catalog for MutableCatalog { match table_info.db_type.clone() { DatabaseType::NormalDB => { info!( - "updating table meta. table desc: [{}], has copied files: [{}]", + "updating table meta. table desc: [{}], has copied files: [{}]?", table_info.desc, req.copied_files.is_some() ); diff --git a/src/query/service/src/interpreters/interpreter_copy.rs b/src/query/service/src/interpreters/interpreter_copy.rs index bbadab89dca81..f00205ca73379 100644 --- a/src/query/service/src/interpreters/interpreter_copy.rs +++ b/src/query/service/src/interpreters/interpreter_copy.rs @@ -433,12 +433,11 @@ impl CopyInterpreter { let table_id = to_table.get_id(); let expire_hours = ctx.get_settings().get_load_file_metadata_expire_hours()?; - let table_info = to_table.get_table_info(); let upsert_copied_files_request = { if stage_info.copy_options.purge && force { // if `purge-after-copy` is enabled, and in `force` copy mode, // we do not need to upsert copied files into meta server - info!("[purge] and [force] are both enabled, will not update copied-files set. ({})", &table_info.desc); + info!("[purge] and [force] are both enabled, will not update copied-files set. ({})", &to_table.get_table_info().desc); None } else { let fail_if_duplicated = !force;