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.

Sign up for GitHub

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

Jump to bottom

[WIP] compactor: Sets block size compaction limit #3238

Closed
wants to merge 2 commits into from
Closed

[WIP] compactor: Sets block size compaction limit #3238

wants to merge 2 commits into from

Conversation

someshkoli
Copy link
Contributor

Currently WIP and aims to resolve #3068
Partially solves #3221 as block size is required here too.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

@someshkoli
Adds method to get block size
8f7d6cf 
 - adds new size struct which represents size of a block. contains index
   size and chunk sie
 - adds GetSize method which taked block id and returns Size object
   containing index size and block size
@someshkoli
Adds check for oversized block index
cb990aa 
 - Adds checkBlockIndexSize metho to check size of block index
 - Adds sizeCheck method which check wether input size is greater than
   64 GiB
 - Adds check for creating new meta.json directory for new blocks
 - Creates new plan according to the 64 GiB block index size filter
@bwplotka
Copy link
Member

Amazing, thanks for helping on this - we need help here.

Wonder what's the plan here? Detecting size is important, but how we ensure proper sharding of block? (:

@someshkoli
Copy link
Contributor Author

someshkoli commented Sep 28, 2020 •
edited
Loading

@bwplotka, As for now I'm done limiting compaction acc to the block index size.
For sharding I'm planning to do the following, would improvise on this, also let me know if the implementation is wrong

  • compact a blocks
  • check final block index size (newly generated)
  • if greater than x(64 gib for now)
    • partition it in two roughly equal parts ( do this recursively till all the block index sizes are < x )
    • write new blocks and move forward
  • if less than 64 move forward as usual

This way our blocks are compacted to its maximum and also we prevent it from exceeding the limit

@roidelapluie
Copy link

Note that recently Prometheus BlockReader gained Size() function.

@someshkoli
Copy link
Contributor Author

Note that recently Prometheus BlockReader gained Size() function.

yeah I did pick that up (disscussed here: #3254) but having some trouble in testing the changes on the same.

@bwplotka
Copy link
Member

I am moving with #3068 and will probably use the code from this PR if you don't mind 🤗

@someshkoli
Copy link
Contributor Author

I am moving with #3068 and will probably use the code from this PR if you don't mind 🤗

Yes sure ✌️

@bwplotka
Copy link
Member

Awesome, closing for now then, but will use your code (:

@bwplotka bwplotka closed this Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Limit size of blocks to X bytes on compaction.
3 participants
@someshkoli @bwplotka @roidelapluie