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

feat: another format for manifest #1604

Merged
merged 6 commits into from
Dec 12, 2024

Conversation

zealchen
Copy link
Contributor

@zealchen zealchen commented Dec 6, 2024

Rationale

Close #1600

Detailed Changes

Creating a new data format to represent the manifest snapshot file.

| magic(u32) | version(u8) |  flags(u8) | length(u64) | Record(N) ... |

The Magic field (u32) is used to ensure the validity of the data source.
The Flags field (u8) is reserved for future extensibility, such as enabling compression or supporting additional features.
The length field (u64) represents the total length of the subsequent records and serves as a straightforward method for verifying their integrity. (length = record_length * record_count)

# Record is a self-descriptive message
| id(u64) | time_range(i64*2)| size(u32) |  num_rows(u32)|

In do_merge, the snapshot data handle is like:

Old data flow in do_merge:
                                      delta_sstmetas
                                             | (extend vec)
                                             V                                
object_store -> org_bytes -> org_pb -> Vec<sstmeta> -> dst_pb -> dst_bytes -> object_store

New data flow in do_merge:
               delta_sstmetas -> bytes
                                  | (append)
                                  V                                
object_store -> org_bytes -> dst_bytes -> object_store

Specifically, I create the SnapshotHeader and SnapshotRecordV1 to represent the corresponding data in snapshot bytes. Before merging delta sstfiles into new bytes, we allocate a larger Vec <u8> and copy each segment (header, old records, new records) into it.

This RP DOES NOT address format upgrade logic which can be resolved in a separate PR. As for the upgrade, we could define a new SnapshotRecord format and perform data migration in Manifest::try_new.

Test Plan

UT

@github-actions github-actions bot added the feature New feature or request label Dec 6, 2024
@jiacai2050 jiacai2050 self-requested a review December 10, 2024 06:26
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
horaedb/metric_engine/src/manifest.rs Outdated Show resolved Hide resolved
@zealchen zealchen requested a review from jiacai2050 December 12, 2024 04:24
Copy link
Contributor

@jiacai2050 jiacai2050 left a comment

Choose a reason for hiding this comment

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

LGTM

@jiacai2050 jiacai2050 merged commit 25dbe73 into apache:main Dec 12, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explore another format for manifest
2 participants