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

Add estimated_size for Block structure #130

Open
ppdogg opened this issue Mar 4, 2025 · 4 comments
Open

Add estimated_size for Block structure #130

ppdogg opened this issue Mar 4, 2025 · 4 comments

Comments

@ppdogg
Copy link
Contributor

ppdogg commented Mar 4, 2025

There has already been a function estimated_size in SsTableBuilder. I think it is fair to add the same function to BlockBuilder structure:

impl BlockBuilder {
     pub fn estimated_size(&self) -> usize {}
}

Doing this has two benefits:

  1. Making the estimated_size in SsTableBuilder more precise:
pub fn estimated_size(&self) -> usize {
    self.data.len() + self.builder.estimated_size()
}

Because during the process of building table, there may be some data in block builder that are not taken into count.

  1. In function compact of compact.rs, we can use estimated_size of SsTableBuilder to find if there are some data left:
if builder.estimated_size() > 0 {}

I have read your code in checkpoint, and I think this eliminates the need to use Option as a wrapper of SsTableBuilder.

@ppdogg
Copy link
Contributor Author

ppdogg commented Mar 4, 2025

Also: In concat_iterator.rs you use next_sst_idx; In lsm_storage.rs, you use next_sst_id

I think it'd better to use the same name.

@ppdogg
Copy link
Contributor Author

ppdogg commented Mar 4, 2025

PS: Maybe you could add a Q&A in each section. There must be some questions that have been frequently asked, which you've already answered in discord.

@skyzh
Copy link
Owner

skyzh commented Mar 9, 2025

+1 :) I'd like to postpone API updates to the next major update of the course. I'll leave this open so that I don't forget.

@ppdogg
Copy link
Contributor Author

ppdogg commented Mar 11, 2025

+1 :) I'd like to postpone API updates to the next major update of the course. I'll leave this open so that I don't forget.

If you want to add estimate_size for BlockBuilder, there is something you need to modify:

pub fn estimated_size(&self) -> usize {
    self.data.len() + SIZEOF_U16 * self.offsets.len()
}

rather than

pub fn estimated_size(&self) -> usize {
    self.data.len() + SIZEOF_U16 * self.offsets.len() + SIZEOF_U16
}

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

No branches or pull requests

2 participants