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 without tombstone #33

Merged
merged 12 commits into from
Nov 7, 2024
Merged

Conversation

sebheitzmann
Copy link
Contributor

@sebheitzmann sebheitzmann commented Sep 16, 2024

Pull request to merge sstables without tombstone.

Todo list :

  • create a test in compaction_test to check the removal of tombstoned value
  • correct compation
  • add a mechanism to garbage collect outside of compaction ( or force a compact )

@sebheitzmann sebheitzmann changed the title Compaction without thumbstone Compaction without tombstone Sep 16, 2024
Add TombRecords and size in sstable metadata to handle the GC process with a Threshold in size or record nb.
@thomasjungblut
Copy link
Owner

great start @sebheitzmann

@thomasjungblut
Copy link
Owner

do you need anything from me? feel free to ping me once you're ready :)

@sebheitzmann
Copy link
Contributor Author

I think that it's ok for me. The Tombstone are correctely merge. But only if we do a "full" compaction. If not, we cannot be sure that there is no previous version in the sst not selected for merge. I think that to go further we need some sort of layer merging that is not implemented for the moment. Do you think that this can be merge for the moment ?

@thomasjungblut
Copy link
Owner

Yes, let me give it one last look. Then we can merge.

@sebheitzmann
Copy link
Contributor Author

sebheitzmann commented Nov 7, 2024

For my usage i will certainely do a merge with layer calculated by an key préfix. In my software the key are already a Hash ( sha1 ). So y can do some folder based on the first 4 caracters of the hash for exemple and compact the layer 0 on this subfolder. After that the layers 1 can be full compacted without risk.

But my usage is not really standard

@thomasjungblut
Copy link
Owner

code looks good, let's see if there are any failures on CI

After that the layers 1 can be full compacted without risk.

Yeah I didn't get to implement a full "LSM Tree" kind of merge layer. Glad you found some use of it though and definitely thank you tons for contributing :)

@thomasjungblut
Copy link
Owner

off we go! Thank you very much @sebheitzmann - I'll briefly need to log into my other machine to prep a new minor release, will get that out asap.

@thomasjungblut thomasjungblut merged commit 0ca9659 into thomasjungblut:main Nov 7, 2024
4 checks passed
@thomasjungblut
Copy link
Owner

Done!

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