ISSUE-2361: remote backend for fuse table #2355

dantengsky · 2021-10-20T15:08:43Z

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

Summary

simple impl of the remote backend for fuse table
recovery integration tests '^09_*'

for reviewers:

two additional match branches have been added to DataValue::try_into_data_array
test cases seem work, but I am not sure if they are proper
metadata is updated without consensus, will be fixed in latter PR

Changelog

Improvement
Not for changelog (changelog entry is not required)

Related Issues

Fixes: #2361

Test Plan

Unit Tests

Stateless Tests

databend-bot · 2021-10-20T15:08:46Z

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.

codecov-commenter · 2021-10-20T15:34:45Z

Codecov Report

Merging #2355 (b8b128e) into main (8107725) will decrease coverage by 0%.
The diff coverage is 30%.

@@          Coverage Diff          @@
##            main   #2355   +/-   ##
=====================================
- Coverage     71%     71%   -1%     
=====================================
  Files        633     633           
  Lines      35703   35754   +51     
=====================================
- Hits       25436   25428    -8     
- Misses     10267   10326   +59

Impacted Files	Coverage Δ
common/datavalues/src/data_value_ops.rs	`40% <0%> (-3%)`	⬇️
common/meta/raft-store/src/state_machine/sm.rs	`95% <ø> (ø)`
metasrv/src/executor/action_handler.rs	`92% <0%> (-4%)`	⬇️
metasrv/src/meta_service/raftmeta.rs	`87% <0%> (-3%)`	⬇️
query/src/catalogs/backends/impls/meta_cached.rs	`0% <0%> (ø)`
query/src/catalogs/backends/impls/meta_remote.rs	`0% <0%> (ø)`
query/src/catalogs/backends/impls/meta_sync.rs	`0% <0%> (ø)`
...uery/src/catalogs/backends/impls/remote_backend.rs	`0% <0%> (ø)`
query/src/catalogs/catalog.rs	`0% <ø> (ø)`
query/src/catalogs/impls/catalog/system_catalog.rs	`77% <0%> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8107725...b8b128e. Read the comment docs.

BohuTANG · 2021-10-20T23:49:30Z

This PR needs an issue to track, for example in our release proposal :)

dantengsky · 2021-10-21T02:16:31Z

This PR needs an issue to track, for example in our release proposal :)

got it

sundy-li · 2021-10-21T03:54:56Z

common/datavalues/src/data_value_ops.rs

@@ -85,6 +85,12 @@ impl DataValue {
            }
            DataType::Boolean => try_build_array! {values},
            DataType::String => try_build_array! {String, values},
+            DataType::Date16 => {


Miss Date32

sundy-li · 2021-10-21T03:55:31Z

common/datavalues/src/macros.rs

@@ -186,7 +186,10 @@ macro_rules! try_build_array {
            match value {
                DataValue::$SCALAR_TY(Some(v)) => builder.append_value(*v),
                DataValue::$SCALAR_TY(None) => builder.append_null(),
-                _ => unreachable!(),
+                dv => {


unreachable!(dv) ?

oops, forget code GC here

dantengsky · 2021-10-21T04:20:01Z

/cc @sundy-li PTAL

drmingdrmer · 2021-10-21T04:58:44Z

common/meta/api/src/meta_api.rs

+    async fn upsert_table_option(
        &self,
        table_id: MetaId,
-        new_table_version: MetaVersion,
-        new_snapshot_location: String,
-    ) -> Result<CommitTableReply>;
+        table_version: MetaVersion,
+        option_key: String,
+        option_value: String,
+    ) -> Result<UpsertTableOptionReply>;


Why not just define a upsert_table, to update an entire table instance?

An update of a single field is only useful with non-serialized concurrency control. E.g. two concurrent commutative update operations to a single table.

As MetaVersion must be matched, it is serialized. Updating the entire table simplifies the backend impl.

right, that is more reasonable. let's fix this in latter prs

Then everything else looks good enough to me.

BohuTANG · 2021-10-21T06:53:25Z

/lgtm

Great, welcome fuse table come back.

databend-bot · 2021-10-21T06:53:27Z

Approved! Thank you for the PR @dantengsky

databend-bot · 2021-10-21T06:53:44Z

CI Passed
Reviewer Approved
Let's Merge

drmingdrmer · 2021-10-22T05:48:58Z

BTW, there should be a unit test to the new API upsert_table_option().

databend-bot added pr-improvement labels Oct 20, 2021

BohuTANG mentioned this pull request Oct 20, 2021

Release proposal: Nightly v0.5 #2257

Closed

dantengsky changed the title ~~remote backend for fuse table~~ ISSUE-2361: remote backend for fuse table Oct 21, 2021

remote backend for fuse table

351fc51

dantengsky force-pushed the feature-remote-backend branch from eedcd41 to 351fc51 Compare October 21, 2021 03:20

dantengsky marked this pull request as ready for review October 21, 2021 03:40

dantengsky requested review from sundy-li, drmingdrmer, BohuTANG and ariesdevil October 21, 2021 03:41

sundy-li reviewed Oct 21, 2021

View reviewed changes

add DataType::Data32 for try_into_data_array

b8b128e

drmingdrmer reviewed Oct 21, 2021

View reviewed changes

drmingdrmer mentioned this pull request Oct 21, 2021

Why not just define a upsert_table, to update an entire table instance? #2368

Closed

databend-bot approved these changes Oct 21, 2021

View reviewed changes

databend-bot merged commit 2b9cd07 into databendlabs:main Oct 21, 2021

drmingdrmer mentioned this pull request Oct 22, 2021

There should be a unit test to the new API MetaApi::upsert_table_option(). #2392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-2361: remote backend for fuse table #2355

ISSUE-2361: remote backend for fuse table #2355

dantengsky commented Oct 20, 2021 •

edited

Loading

databend-bot commented Oct 20, 2021

codecov-commenter commented Oct 20, 2021 •

edited

Loading

BohuTANG commented Oct 20, 2021

dantengsky commented Oct 21, 2021

sundy-li Oct 21, 2021

dantengsky Oct 21, 2021

sundy-li Oct 21, 2021

dantengsky Oct 21, 2021

dantengsky commented Oct 21, 2021

drmingdrmer Oct 21, 2021

dantengsky Oct 21, 2021

drmingdrmer Oct 21, 2021

BohuTANG commented Oct 21, 2021

databend-bot commented Oct 21, 2021

databend-bot commented Oct 21, 2021

drmingdrmer commented Oct 22, 2021

ISSUE-2361: remote backend for fuse table #2355

ISSUE-2361: remote backend for fuse table #2355

Conversation

dantengsky commented Oct 20, 2021 • edited Loading

Summary

Changelog

Related Issues

Test Plan

databend-bot commented Oct 20, 2021

codecov-commenter commented Oct 20, 2021 • edited Loading

Codecov Report

BohuTANG commented Oct 20, 2021

dantengsky commented Oct 21, 2021

sundy-li Oct 21, 2021

Choose a reason for hiding this comment

dantengsky Oct 21, 2021

Choose a reason for hiding this comment

sundy-li Oct 21, 2021

Choose a reason for hiding this comment

dantengsky Oct 21, 2021

Choose a reason for hiding this comment

dantengsky commented Oct 21, 2021

drmingdrmer Oct 21, 2021

Choose a reason for hiding this comment

dantengsky Oct 21, 2021

Choose a reason for hiding this comment

drmingdrmer Oct 21, 2021

Choose a reason for hiding this comment

BohuTANG commented Oct 21, 2021

databend-bot commented Oct 21, 2021

databend-bot commented Oct 21, 2021

drmingdrmer commented Oct 22, 2021

dantengsky commented Oct 20, 2021 •

edited

Loading

codecov-commenter commented Oct 20, 2021 •

edited

Loading