Skip to content

feat: introduce snapshot manager#103

Merged
JingsongLi merged 1 commit intoapache:mainfrom
luoyuxia:introduce-snapshot-manager1
Mar 3, 2026
Merged

feat: introduce snapshot manager#103
JingsongLi merged 1 commit intoapache:mainfrom
luoyuxia:introduce-snapshot-manager1

Conversation

@luoyuxia
Copy link
Contributor

@luoyuxia luoyuxia commented Mar 3, 2026

Purpose

Linked issue: close #30

Brief change log

Tests

API and Format

Documentation

@luoyuxia luoyuxia force-pushed the introduce-snapshot-manager1 branch from fcb0d61 to a54fdd8 Compare March 3, 2026 02:19

/// Get the latest snapshot, or None if LATEST does not exist.
/// Returns an error if LATEST exists but the snapshot file (snapshot-{id}) does not exist.
pub async fn get_latest_snapshot(&self) -> crate::Result<Option<Snapshot>> {

Choose a reason for hiding this comment

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

Suggest adding unit tests for get_latest_snapshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was intending to add, but found it's not a simple work since it'll require write to tables. I think we can add when we support write to tables.


/// Path to the snapshot directory (e.g. `table_path/snapshot`).
pub fn snapshot_dir(&self) -> String {
format!("{}/{}", self.table_path, SNAPSHOT_DIR)

Choose a reason for hiding this comment

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

Does this work well in Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should(but i can't give a try) since rust's Path will handle the path across different operator systems. See
https://doc.rust-lang.org/std/path/struct.Path.html

This type supports a number of operations for inspecting a path, including breaking the path into its components (separated by / on Unix and by either / or \ on Windows), extracting the file name, determining whether the path is absolute, and so on.

@XiaoHongbo-Hope
Copy link

+1

@JingsongLi
Copy link
Contributor

+1

@JingsongLi JingsongLi merged commit 96c39f2 into apache:main Mar 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Impl SnapshotManager

3 participants