Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: add DatabaseMeta.gc_in_progress #16464

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/meta/api/src/name_id_value_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
24 changes: 23 additions & 1 deletion src/meta/app/src/schema/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,29 @@ pub struct DatabaseMeta {
pub updated_on: DateTime<Utc>,
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<DateTime<Utc>>,

/// 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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means all dbs in old version this flag is false?

Because these dbs are not in progress.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Currently no db is in gc state, this new state will be set only when the gc is started. And current codebase does not use this state.

}

impl Default for DatabaseMeta {
Expand All @@ -83,6 +104,7 @@ impl Default for DatabaseMeta {
updated_on: Utc::now(),
comment: "".to_string(),
drop_on: None,
gc_in_progress: false,
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/meta/proto-conv/src/database_from_to_protobuf_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ impl FromToProto for mt::DatabaseMeta {
Some(drop_on) => Some(DateTime::<Utc>::from_pb(drop_on)?),
None => None,
},
gc_in_progress: p.gc_in_progress,
comment: p.comment,
};
Ok(v)
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 2 additions & 0 deletions src/meta/proto-conv/tests/it/proto_conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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,
}
}

Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v002_database_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v005_database_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v055_table_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v074_table_db_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v096_database_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/v101_database_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
60 changes: 60 additions & 0 deletions src/meta/proto-conv/tests/it/v110_database_meta_gc_in_progress.rs
Original file line number Diff line number Diff line change
@@ -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_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, 110, 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_v110.as_slice(), 110, want())
}

fn s(ss: impl ToString) -> String {
ss.to_string()
}
5 changes: 5 additions & 0 deletions src/meta/protos/proto/database.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading