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

Compaction for fragmented pages #3

Merged
merged 9 commits into from
Sep 27, 2024
Merged

Compaction for fragmented pages #3

merged 9 commits into from
Sep 27, 2024

Conversation

coszio
Copy link
Contributor

@coszio coszio commented Sep 25, 2024

After a while, we expect that having enough updates will create unused regions in between the data for the values, which will be wasted space.

With these changes we add the ability to trigger a compaction. It:

  • Detects which pages are over some threshold of fragmentation
  • Copies all values in these pages into new pages sequentially, so that the new pages will have perfectly contiguous data.
  • Introduces the point id into the slot, so that we can update the page_tracker easily.
  • Updates the page_tracker with the new pointer to the slot in the new page.
  • Deletes the fragmented pages.

@coszio coszio marked this pull request as draft September 25, 2024 02:05
@agourlay
Copy link
Member

Please rebase on master to get CI 🔥

@coszio coszio marked this pull request as ready for review September 26, 2024 14:20
// This is a loop because we might need to create more pages if the current new page is full
'new_page: loop {
// create new page
let new_page_id = self.create_new_page(size_hint);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should create this page as a tempfile and then move it at the end of compaction.
This way there is no left over if compaction is interrupted.

}

/// Sums the amount of unused space in between the data.
pub fn fragmented_space(&self) -> usize {
Copy link
Member

Choose a reason for hiding this comment

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

Can you maybe write a simple test for this one?
Write a few values and then delete one and check that fragmentation increases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, I was forgetting about this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (ced1918)

Copy link
Member

@agourlay agourlay left a comment

Choose a reason for hiding this comment

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

Decided to merge fast to play with it without having the burden to rebase.

@agourlay agourlay merged commit bc5cb54 into master Sep 27, 2024
2 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.

2 participants