Feature: add metasrv time travel functions #5468

lichuang · 2022-05-19T09:57:01Z

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

add metasrv time travel functions
add get_{db|table}_history in schema API.

Changelog

New Feature

Related Issues

Part of #5572

vercel · 2022-05-19T09:57:10Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
databend	⬜️ Ignored (Inspect)		May 24, 2022 at 8:31AM (UTC)

mergify · 2022-05-19T09:57:34Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

drmingdrmer · 2022-05-23T03:21:50Z

common/meta/api/src/schema_api.rs

+use common_meta_types::UpdateTableMetaReply;
+use common_meta_types::UpdateTableMetaReq;


Is this gonna replace the UpsertTableOption RPC?

Is this gonna replace the UpsertTableOption RPC?

I have no idea, this type is not from this PR, may be ask @dantengsky

Yes, update_table_meta is added in #5476.

dantengsky · 2022-05-24T09:09:48Z

common/meta/api/src/schema_api_impl.rs

+                        txn_op_put(name_key, serialize_id(db_id)?)?, // (tenant, db_name) -> db_id
+                        txn_op_put(&dbid, serialize_struct(&db_meta)?)?, // (db_id) -> db_meta
+                    ],
+                    else_then: vec![],


shall we check the status of name_key in the else branch? so that if name conflict detected, just break out the loop?

Oh, pls ignore it, I miss-interpreted the code

dantengsky · 2022-05-24T09:23:09Z

/LGTM

drmingdrmer · 2022-05-24T09:39:59Z

common/meta/raft-store/tests/it/state_machine/schema_api_impl.rs

+#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+async fn test_meta_embedded_table_drop_undrop_list_history() -> anyhow::Result<()> {
+    let (_log_guards, ut_span) = init_raft_store_ut!();
+    let _ent = ut_span.enter();
+    let tc = new_raft_test_context();
+    let sm = StateMachine::open(&tc.raft_config, 1).await?;
+
+    SchemaApiTestSuite {}
+        .table_drop_undrop_list_history(&sm)
+        .await
+}
+


Need test of database_dorp_undrop_list_history?

fixed in #5566

drmingdrmer · 2022-05-24T09:40:06Z

common/meta/embedded/tests/it/schema_api_impl.rs

+#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+async fn test_meta_embedded_table_drop_undrop_list_history() -> anyhow::Result<()> {
+    let mt = MetaEmbedded::new_temp().await?;
+    SchemaApiTestSuite {}
+        .table_drop_undrop_list_history(&mt)
+        .await
+}
+


Need test of database_dorp_undrop_list_history?

fixed in #5566

drmingdrmer · 2022-05-24T09:52:09Z

common/meta/api/src/schema_api_impl.rs

+                return Err(MetaError::AppError(AppError::UndropDbAlreadyExists(
+                    UndropDbAlreadyExists::new(&name_key.db_name),
+                )));


Would it be simpler if just using DatabaseAlreadyExists instead of UndropDbAlreadyExists?

fixed in #5566

drmingdrmer · 2022-05-24T09:53:45Z

common/meta/api/src/schema_api_impl.rs

+                }
+            };
+
+            // get tb_meta of the last db id


Suggested change

// get tb_meta of the last db id

// get db_meta of the last db id

fixed in #5566

drmingdrmer · 2022-05-24T10:04:00Z

common/meta/api/src/schema_api_impl.rs

+            let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
+                get_struct_value(self, &dbid_idlist).await?;
+            let mut db_id_list: DbIdList;
+            if db_id_list_seq == 0 {
+                db_id_list = DbIdList::new();
+                db_id_list.append(old_db_id);


If the db exists but the db_id_list does not, it must be a bug and an error should be returned. Unless metasrv allows removing a db_id_list record while the db it belongs to still exists.

there is a case that: already create a {db|table} before we add time travel related function, thus these {db|table} is not in id list, so we must handle this case.

drmingdrmer · 2022-05-24T10:05:13Z

common/meta/api/src/schema_api_impl.rs

+            let (new_db_id_list_seq, new_db_id_list_opt): (_, Option<DbIdList>) =
+                get_struct_value(self, &new_dbid_idlist).await?;
+            let mut new_db_id_list: DbIdList;
+            if new_db_id_list_seq == 0 {
+                new_db_id_list = DbIdList::new();


The same as the previous comment.

drmingdrmer · 2022-05-24T10:24:20Z

common/meta/api/src/schema_api_impl.rs

+                db_id_list.pop();
+                new_db_id_list.append(old_db_id);


db_id_list feels like a hash-map, but not a list, if it is allowed to undrop a db whose id is not at the top in db_id_list.

currently, undrop only the top in db_id_list.

drmingdrmer · 2022-05-24T10:35:36Z

common/meta/api/src/schema_api_impl.rs

+                match db_id_list_opt {
+                    Some(list) => list,
+                    None => {
+                        return Err(MetaError::AppError(AppError::UndropDbHasNoHistory(
+                            UndropDbHasNoHistory::new(&name_key.db_name),
+                        )))
+                    }
+                }


Reading the db_id and reading the db_id_list is not guaranteed to happen in a snapshot.
Thus there is a chance db_id_list is inconsistent and in such a case it should retry in next loop.

Feature: add metasrv time travel functions

ef2c589

lichuang requested a review from BohuTANG as a code owner May 19, 2022 09:57

databend-bot added the need-review label May 19, 2022

mergify bot added the pr-feature this PR introduces a new feature to the codebase label May 19, 2022

lichuang requested a review from drmingdrmer May 19, 2022 09:58

lichuang added the A-meta Area: databend meta serive label May 19, 2022

lichuang requested a review from dantengsky May 19, 2022 10:07

merge from upstream/main

0733be8

vercel bot deployed to Preview May 23, 2022 02:46 View deployment

drmingdrmer reviewed May 23, 2022

View reviewed changes

lichuang added 4 commits May 24, 2022 16:20

Feature: add metasrv time travel functions, add unit tests

8fa8074

Merge remote-tracking branch 'upstream/main' into meta_time_travel

57fcde0

merge from upstream/main

1038cf9

fix clippy format bug

3d1f39c

dantengsky reviewed May 24, 2022

View reviewed changes

databend-bot approved these changes May 24, 2022

View reviewed changes

BohuTANG merged commit b80625e into databendlabs:main May 24, 2022

drmingdrmer reviewed May 24, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: add metasrv time travel functions #5468

Feature: add metasrv time travel functions #5468

lichuang commented May 19, 2022 •

edited

Loading

vercel bot commented May 19, 2022 •

edited

Loading

mergify bot commented May 19, 2022

drmingdrmer May 23, 2022

lichuang May 24, 2022

dantengsky May 24, 2022

dantengsky May 24, 2022

dantengsky May 24, 2022

dantengsky commented May 24, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

drmingdrmer May 24, 2022

lichuang May 25, 2022

drmingdrmer May 24, 2022

		use common_meta_types::UpdateTableMetaReply;
		use common_meta_types::UpdateTableMetaReq;

	// get tb_meta of the last db id
	// get db_meta of the last db id

Feature: add metasrv time travel functions #5468

Feature: add metasrv time travel functions #5468

Conversation

lichuang commented May 19, 2022 • edited Loading

Summary

Changelog

Related Issues

vercel bot commented May 19, 2022 • edited Loading

mergify bot commented May 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dantengsky commented May 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lichuang commented May 19, 2022 •

edited

Loading

vercel bot commented May 19, 2022 •

edited

Loading