From 9e8bb7d04d2778536ceba666b8b14ccbb10275bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Wed, 18 Sep 2024 21:30:13 +0800 Subject: [PATCH 1/2] refactor: add DatabaseMeta.gc_in_progress If `gc_in_progress` is set, no un-drop can be done on this database. Because the related data may already have been deleted. This commit does not use this flag yet, and will be used in next commit. - Part of #16433 --- src/meta/api/src/name_id_value_api.rs | 1 + src/meta/app/src/schema/database.rs | 24 +++++++- .../src/database_from_to_protobuf_impl.rs | 2 + src/meta/proto-conv/src/util.rs | 1 + src/meta/proto-conv/tests/it/main.rs | 1 + src/meta/proto-conv/tests/it/proto_conv.rs | 2 + .../proto-conv/tests/it/v002_database_meta.rs | 1 + .../proto-conv/tests/it/v005_database_meta.rs | 1 + .../proto-conv/tests/it/v055_table_meta.rs | 1 + .../proto-conv/tests/it/v074_table_db_meta.rs | 1 + .../proto-conv/tests/it/v096_database_meta.rs | 1 + .../proto-conv/tests/it/v101_database_meta.rs | 1 + .../it/v110_database_meta_gc_in_progress.rs | 60 +++++++++++++++++++ src/meta/protos/proto/database.proto | 5 ++ 14 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs diff --git a/src/meta/api/src/name_id_value_api.rs b/src/meta/api/src/name_id_value_api.rs index 8b705272f0f6..6309fe40330f 100644 --- a/src/meta/api/src/name_id_value_api.rs +++ b/src/meta/api/src/name_id_value_api.rs @@ -449,6 +449,7 @@ mod tests { updated_on: Default::default(), comment: "".to_string(), drop_on: None, + gc_in_progress: false, }; let v = db_meta(1).to_pb()?.encode_to_vec(); diff --git a/src/meta/app/src/schema/database.rs b/src/meta/app/src/schema/database.rs index 04dd518412fc..e5997e5416ad 100644 --- a/src/meta/app/src/schema/database.rs +++ b/src/meta/app/src/schema/database.rs @@ -69,8 +69,29 @@ pub struct DatabaseMeta { pub updated_on: DateTime, pub comment: String, - // if used in CreateDatabaseReq, this field MUST set to None. + /// if used in CreateDatabaseReq, this field MUST set to None. pub drop_on: Option>, + + /// Indicates whether garbage collection is currently in progress for this dropped database. + /// + /// If it is in progress, the database should not be un-dropped, because the data may be incomplete. + /// + /// ```text + /// normal <----. + /// | | + /// | drop() | undrop() + /// v | + /// dropped ----' + /// | + /// | gc() + /// v + /// gc_in_progress=True + /// | + /// | purge data from meta-service + /// v + /// completed removed + /// ``` + pub gc_in_progress: bool, } impl Default for DatabaseMeta { @@ -83,6 +104,7 @@ impl Default for DatabaseMeta { updated_on: Utc::now(), comment: "".to_string(), drop_on: None, + gc_in_progress: false, } } } diff --git a/src/meta/proto-conv/src/database_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/database_from_to_protobuf_impl.rs index f4d533d98aa2..94c186b7c6e6 100644 --- a/src/meta/proto-conv/src/database_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/database_from_to_protobuf_impl.rs @@ -44,6 +44,7 @@ impl FromToProto for mt::DatabaseMeta { Some(drop_on) => Some(DateTime::::from_pb(drop_on)?), None => None, }, + gc_in_progress: p.gc_in_progress, comment: p.comment, }; Ok(v) @@ -62,6 +63,7 @@ impl FromToProto for mt::DatabaseMeta { Some(drop_on) => Some(drop_on.to_pb()?), None => None, }, + gc_in_progress: self.gc_in_progress, comment: self.comment.clone(), shared_by: vec![], from_share: None, diff --git a/src/meta/proto-conv/src/util.rs b/src/meta/proto-conv/src/util.rs index d685d83b22ba..b86e0b8a37a6 100644 --- a/src/meta/proto-conv/src/util.rs +++ b/src/meta/proto-conv/src/util.rs @@ -139,6 +139,7 @@ const META_CHANGE_LOG: &[(u64, &str)] = &[ (107, "2024-08-09: Add: datatype.proto/DataType Geography type"), (108, "2024-08-29: Add: procedure.proto: ProcedureMeta and ProcedureIdentity"), (109, "2024-08-29: Refactor: ProcedureMeta add arg_names"), + (110, "2024-09-18: Add: database.proto: DatabaseMeta.gc_in_progress"), // Dear developer: // If you're gonna add a new metadata version, you'll have to add a test for it. // You could just copy an existing test file(e.g., `../tests/it/v024_table_meta.rs`) diff --git a/src/meta/proto-conv/tests/it/main.rs b/src/meta/proto-conv/tests/it/main.rs index 5d705b6aa305..2ab430fb1ad1 100644 --- a/src/meta/proto-conv/tests/it/main.rs +++ b/src/meta/proto-conv/tests/it/main.rs @@ -107,3 +107,4 @@ mod v106_query_token; mod v107_geography_datatype; mod v108_procedure; mod v109_procedure_with_args; +mod v110_database_meta_gc_in_progress; diff --git a/src/meta/proto-conv/tests/it/proto_conv.rs b/src/meta/proto-conv/tests/it/proto_conv.rs index 3deb9f42a37d..bbf33928eb39 100644 --- a/src/meta/proto-conv/tests/it/proto_conv.rs +++ b/src/meta/proto-conv/tests/it/proto_conv.rs @@ -52,6 +52,7 @@ fn new_db_meta_share() -> mt::DatabaseMeta { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, } } @@ -64,6 +65,7 @@ fn new_db_meta() -> mt::DatabaseMeta { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, } } diff --git a/src/meta/proto-conv/tests/it/v002_database_meta.rs b/src/meta/proto-conv/tests/it/v002_database_meta.rs index 914b08432430..802f6c5a3e7c 100644 --- a/src/meta/proto-conv/tests/it/v002_database_meta.rs +++ b/src/meta/proto-conv/tests/it/v002_database_meta.rs @@ -48,6 +48,7 @@ fn test_decode_v2_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v005_database_meta.rs b/src/meta/proto-conv/tests/it/v005_database_meta.rs index 8cc7cb67b5ae..515b67f0dfc8 100644 --- a/src/meta/proto-conv/tests/it/v005_database_meta.rs +++ b/src/meta/proto-conv/tests/it/v005_database_meta.rs @@ -49,6 +49,7 @@ fn test_decode_v5_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v055_table_meta.rs b/src/meta/proto-conv/tests/it/v055_table_meta.rs index 879d18ade2e4..6d10be03331e 100644 --- a/src/meta/proto-conv/tests/it/v055_table_meta.rs +++ b/src/meta/proto-conv/tests/it/v055_table_meta.rs @@ -119,6 +119,7 @@ fn test_decode_v51_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v074_table_db_meta.rs b/src/meta/proto-conv/tests/it/v074_table_db_meta.rs index d207108cb274..39a54e2f4cdc 100644 --- a/src/meta/proto-conv/tests/it/v074_table_db_meta.rs +++ b/src/meta/proto-conv/tests/it/v074_table_db_meta.rs @@ -116,6 +116,7 @@ fn test_decode_v74_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v096_database_meta.rs b/src/meta/proto-conv/tests/it/v096_database_meta.rs index 1ef71b3b6d83..fb9eec2a66e0 100644 --- a/src/meta/proto-conv/tests/it/v096_database_meta.rs +++ b/src/meta/proto-conv/tests/it/v096_database_meta.rs @@ -39,6 +39,7 @@ fn test_decode_v96_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v101_database_meta.rs b/src/meta/proto-conv/tests/it/v101_database_meta.rs index 83485bdf8583..ab36a56389b1 100644 --- a/src/meta/proto-conv/tests/it/v101_database_meta.rs +++ b/src/meta/proto-conv/tests/it/v101_database_meta.rs @@ -40,6 +40,7 @@ fn v101_database_meta() -> anyhow::Result<()> { updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), comment: "foo bar".to_string(), drop_on: None, + gc_in_progress: false, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs b/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs new file mode 100644 index 000000000000..784c88ee54c3 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs @@ -0,0 +1,60 @@ +// Copyright 2023 Datafuse Labs. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use chrono::TimeZone; +use chrono::Utc; +use databend_common_meta_app::schema as mt; +use fastrace::func_name; +use maplit::btreemap; + +use crate::common; + +// These bytes are built when a new version in introduced, +// and are kept for backward compatibility test. +// +// ************************************************************* +// * These messages should never be updated, * +// * only be added when a new version is added, * +// * or be removed when an old version is no longer supported. * +// ************************************************************* +// +// The message bytes are built from the output of `test_pb_from_to()` +#[test] +fn test_decode_v109_database_meta() -> anyhow::Result<()> { + let database_meta_v109 = vec![ + 34, 10, 10, 3, 120, 121, 122, 18, 3, 102, 111, 111, 42, 2, 52, 52, 50, 10, 10, 3, 97, 98, + 99, 18, 3, 100, 101, 102, 162, 1, 23, 50, 48, 49, 52, 45, 49, 49, 45, 50, 56, 32, 49, 50, + 58, 48, 48, 58, 48, 57, 32, 85, 84, 67, 170, 1, 23, 50, 48, 49, 52, 45, 49, 49, 45, 50, 57, + 32, 49, 50, 58, 48, 48, 58, 48, 57, 32, 85, 84, 67, 178, 1, 7, 102, 111, 111, 32, 98, 97, + 114, 232, 1, 1, 160, 6, 109, 168, 6, 24, + ]; + + let want = || mt::DatabaseMeta { + engine: "44".to_string(), + engine_options: btreemap! {s("abc") => s("def")}, + options: btreemap! {s("xyz") => s("foo")}, + created_on: Utc.with_ymd_and_hms(2014, 11, 28, 12, 0, 9).unwrap(), + updated_on: Utc.with_ymd_and_hms(2014, 11, 29, 12, 0, 9).unwrap(), + comment: "foo bar".to_string(), + drop_on: None, + gc_in_progress: true, + }; + + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old(func_name!(), database_meta_v109.as_slice(), 109, want()) +} + +fn s(ss: impl ToString) -> String { + ss.to_string() +} diff --git a/src/meta/protos/proto/database.proto b/src/meta/protos/proto/database.proto index ff47219fb813..1af7ee9525d0 100644 --- a/src/meta/protos/proto/database.proto +++ b/src/meta/protos/proto/database.proto @@ -70,6 +70,11 @@ message DatabaseMeta { // The time table dropped. optional string drop_on = 23; + // Indicates whether garbage collection is currently in progress for this dropped database. + // + // If it is in progress, the database should not be un-dropped, because the data may be incomplete. + bool gc_in_progress = 29; + repeated uint64 shared_by = 24; optional TIdent from_share = 25; From cf11f095e13a055cc56b021e40f44e852a3f2138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Thu, 19 Sep 2024 11:42:23 +0800 Subject: [PATCH 2/2] chore: fix up proto tests version: 109->110 --- .../tests/it/v110_database_meta_gc_in_progress.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs b/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs index 784c88ee54c3..23901d89b68a 100644 --- a/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs +++ b/src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs @@ -31,13 +31,13 @@ use crate::common; // // The message bytes are built from the output of `test_pb_from_to()` #[test] -fn test_decode_v109_database_meta() -> anyhow::Result<()> { - let database_meta_v109 = vec![ +fn test_decode_v110_database_meta() -> anyhow::Result<()> { + let database_meta_v110 = vec![ 34, 10, 10, 3, 120, 121, 122, 18, 3, 102, 111, 111, 42, 2, 52, 52, 50, 10, 10, 3, 97, 98, 99, 18, 3, 100, 101, 102, 162, 1, 23, 50, 48, 49, 52, 45, 49, 49, 45, 50, 56, 32, 49, 50, 58, 48, 48, 58, 48, 57, 32, 85, 84, 67, 170, 1, 23, 50, 48, 49, 52, 45, 49, 49, 45, 50, 57, 32, 49, 50, 58, 48, 48, 58, 48, 57, 32, 85, 84, 67, 178, 1, 7, 102, 111, 111, 32, 98, 97, - 114, 232, 1, 1, 160, 6, 109, 168, 6, 24, + 114, 232, 1, 1, 160, 6, 110, 168, 6, 24, ]; let want = || mt::DatabaseMeta { @@ -52,7 +52,7 @@ fn test_decode_v109_database_meta() -> anyhow::Result<()> { }; common::test_pb_from_to(func_name!(), want())?; - common::test_load_old(func_name!(), database_meta_v109.as_slice(), 109, want()) + common::test_load_old(func_name!(), database_meta_v110.as_slice(), 110, want()) } fn s(ss: impl ToString) -> String {