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

upload boltdb files from shipper only when they are not expected to be modified or during shutdown #2487

Conversation

sandeepsukhani
Copy link
Contributor

What this PR does / why we need it:
We want to avoid modifying files once they are uploaded to the store by boltdb-shipper.
This PR takes care of it by avoiding uploading active shard or shard which was active up to a minute back.
It also avoids modifying files created and uploaded by previous restart by considering init time of the Table.

This PR would help build a compactor easily which is much needed since an ingester is now creating 96 files per day which is slowing down the queries.

Checklist

  • Tests updated

@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2020

Codecov Report

Merging #2487 into master will decrease coverage by 0.15%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2487      +/-   ##
==========================================
- Coverage   63.67%   63.52%   -0.16%     
==========================================
  Files         163      163              
  Lines       14325    14334       +9     
==========================================
- Hits         9122     9106      -16     
- Misses       4488     4513      +25     
  Partials      715      715              
Impacted Files Coverage Δ
...kg/storage/stores/shipper/uploads/table_manager.go 68.10% <75.00%> (ø)
pkg/storage/stores/shipper/uploads/table.go 69.87% <83.33%> (+1.72%) ⬆️
pkg/promtail/positions/positions.go 46.49% <0.00%> (-13.16%) ⬇️
pkg/canary/comparator/comparator.go 78.43% <0.00%> (-2.36%) ⬇️
pkg/promtail/targets/file/filetarget.go 66.85% <0.00%> (-2.29%) ⬇️
pkg/logql/evaluator.go 92.47% <0.00%> (-0.41%) ⬇️
pkg/querier/queryrange/downstreamer.go 97.93% <0.00%> (+2.06%) ⬆️

@slim-bean
Copy link
Collaborator

I have one remaining question, if an ingester has been unable to upload tables say because of a network problem and is restarted, I think that means any tables other than the most recent will never be uploaded?

@sandeepsukhani
Copy link
Contributor Author

I have one remaining question, if an ingester has been unable to upload tables say because of a network problem and is restarted, I think that means any tables other than the most recent will never be uploaded?

They would be uploaded because we don't keep a track of which files were uploaded and which weren't so when upload process kicks in it would re-upload everything again.

@slim-bean
Copy link
Collaborator

Ok, this seems ok to me for now, we are only re-uploading completed files which won't be modified again, we are still maintaining our rule of never modfiying a file once uploaded

Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

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

LGTM!

@sandeepsukhani sandeepsukhani force-pushed the avoid-mutating-boltdb-files-in-store branch from 542fc2b to 993f5b6 Compare August 24, 2020 11:33
@sandeepsukhani sandeepsukhani merged commit 9b0740c into grafana:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants