Skip to content

Conversation

chenzl25
Copy link
Collaborator

@chenzl25 chenzl25 commented Mar 4, 2025

@chenzl25 chenzl25 requested review from ZENOTME and Li0k March 4, 2025 07:22
commit_uuid: Option<Uuid>,
key_metadata: Vec<u8>,
) -> Result<FastAppendAction<'a>> {
let snapshot_id = self.generate_unique_snapshot_id();
let snapshot_id = if let Some(snapshot_id) = snapshot_id {
snapshot_id
Copy link

Choose a reason for hiding this comment

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

We should ensure the snapshot id is unique in snapshots. Otherwise we should return the error and let client to regenerate it.

fn generate_unique_snapshot_id(&self) -> i64 {
       ...
        let mut snapshot_id = generate_random_id();
        while self
            .table
            .metadata()
            .snapshots()
            .any(|s| s.snapshot_id() == snapshot_id)
        {
            snapshot_id = generate_random_id();
        }
        snapshot_id
    }

@chenzl25 chenzl25 requested a review from ZENOTME March 4, 2025 08:00
@chenzl25
Copy link
Collaborator Author

chenzl25 commented Mar 5, 2025

cc @ZENOTME PTAL

Copy link

@ZENOTME ZENOTME left a comment

Choose a reason for hiding this comment

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

LGTM!

@chenzl25 chenzl25 merged commit bc067f0 into dev_rebase_main_20241230 Mar 5, 2025
1 check passed
chenzl25 added a commit that referenced this pull request Mar 12, 2025
* allow specify snapshot id for fast append
chenzl25 added a commit that referenced this pull request Mar 12, 2025
* allow specify snapshot id for fast append
chenzl25 added a commit that referenced this pull request Mar 12, 2025
* allow specify snapshot id for fast append (#14)

* allow specify snapshot id for fast append

* fix tests

* fix
xxchan added a commit that referenced this pull request Mar 25, 2025
xxchan added a commit that referenced this pull request Mar 25, 2025
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.

2 participants