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

ISSUE-4241: version of storage layout #4244

Merged
merged 59 commits into from Mar 25, 2022
Merged

Conversation

dantengsky
Copy link
Member

@dantengsky dantengsky commented Feb 25, 2022

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

Summary

Issues trying to be addressed in the PR:

  • Table may have multiple format versions of snapshots (time travel)
  • TableSnapshot may have multiple format versions of Segment
  • Segment may also have ...

  • Introduce table option FUSE_OPT_KEY_SNAPSHOT_VER
    which marks the current format version of the snapshot inside meta-server.
  • Enable format version fields in SegmentInfo and TableSnapshot

  • no observable performance degradation in the benchmark of the "ontime" data set

Changelog

  • Improvement
  • Not for changelog (changelog entry is not required)

Related Issues

Fixes #4243

Test Plan

Unit Tests

Stateless Tests

@vercel
Copy link

vercel bot commented Feb 25, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/databend/databend/EMFak54XKZ4RKqCaNhj88i8C57qh
✅ Preview: https://databend-git-fork-dantengsky-issue-4243-databend.vercel.app

[Deployment for 4e90427 canceled]

@mergify
Copy link
Contributor

mergify bot commented Feb 25, 2022

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.

@@ -1,4 +1,4 @@
// Copyright 2021 Datafuse Labs.
// Copyright 2022 Datafuse Labs.
Copy link
Member

Choose a reason for hiding this comment

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

See that we make a new v1 directory for the v1 version, if we only add a new field of the block layout, how about the new directory?
v2/block.rs or ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, there is another versioned mod (but is v0 :D). will be pushed later

Copy link
Member

Choose a reason for hiding this comment

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

😎

@dantengsky dantengsky marked this pull request as ready for review March 25, 2022 10:33
let snapshot = match self.read(loc, None, ver).await {
Ok(s) => s,
Err(e) => {
if e.code() == ErrorCode::dal_path_not_found_code() {
Copy link
Member

Choose a reason for hiding this comment

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

Reminder for myself: #4575

We need to implement From opendal::error::Error so that we can avoid code like this.

use crate::storages::fuse::meta::v0;
use crate::storages::fuse::meta::v1;

pub const CURRNET_SEGMETN_VERSION: u64 = v1::SegmentInfo::VERSION;
Copy link
Member

Choose a reason for hiding this comment

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

Guess CURRNET_* should be CURRENT_*

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, these constants could be removed. these are the legacy code to ensure that for each type of table meta type (e.g. SegmentInfo), there is only one numeric number has been bound to it (the impl Version<n> for SegmentInfo)

impl SnapshotLocationCreator for SnapshotVersion {
fn create(&self, id: &Uuid, prefix: impl AsRef<str>) -> String {
match self {
SnapshotVersion::V0(_) => {
Copy link
Member

Choose a reason for hiding this comment

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

👍
Thank you!

Copy link
Member

@BohuTANG BohuTANG left a comment

Choose a reason for hiding this comment

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

🚀

@BohuTANG BohuTANG merged commit c0e5385 into datafuselabs:main Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add version to storage layout for upgrade
4 participants