Made sure posting preload does not load too much irrelevant bytes. #109
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bwplotka
commented
Dec 8, 2017
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| // Bucket implements the store.Bucket and shipper.Bucket interfaces against local memory. | ||
| type Bucket struct { | ||
| objects map[string][]byte | ||
| dirs map[string]struct{} |
There was a problem hiding this comment.
Sorry, bit unrelated changes, but this actually makes inmem.Bucket behaviour the same as GCS one.
bwplotka
commented
Dec 8, 2017
|
Worth noting that postings are currently also unordered in the index, i.e. postings for the same label name might be scattered around. That amplifies the issue for sure. In the next Prometheus 2 version this is fixed. It's also already fixed for blocks that will run through our compactor. |
fabxc
reviewed
Dec 8, 2017
| @@ -746,6 +751,7 @@ func (b *bucketBlock) readIndexRange(ctx context.Context, off, length int64) ([] | |||
| } | |||
| defer r.Close() | |||
|
|
|||
| // NOTE(bplotka): Huge amount of memory is allocated here. We need to cache it. | |||
There was a problem hiding this comment.
Initially we could address heavy allocs via simple memory pooling instead of actually caching data.
pkg/store/bucket.go
Outdated
| // Maximum amount of irrelevant bytes between postings we are willing to fetch. | ||
| const maxPostingsGap = 1024 | ||
|
|
||
| ctx, cancel := context.WithCancel(ctx) |
There was a problem hiding this comment.
Why are we creating a new context here? It's cancel method is never called either.
There was a problem hiding this comment.
yup - it is used by run.Group below.
pkg/store/bucket.go
Outdated
| }) | ||
|
|
||
| // Maximum amount of irrelevant bytes between postings we are willing to fetch. | ||
| const maxPostingsGap = 1024 |
There was a problem hiding this comment.
This should be way higher. For some manual testing there's basically no difference between fetching 1 byte and 100KB. The per-request overhead outshines significantly here so probably at nearly 500KB of fetching overhead is at least what we want to allow.
|
👍 |
Added comments and restructured preload postings function to allow unit testing. Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
bwplotka
force-pushed
the
preload-postings-opt
branch
from
a8c49f8
to
11360f8
Compare
…il whole request. Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
Made sure single error in query -> storeAPI communication does not fail whole request.
fpetkovski
pushed a commit
to fpetkovski/thanos
that referenced
this pull request
Mar 30, 2023
* Native histogram downsampling Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixed changelog Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Updated Readme Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Add chnagelog Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixed changelog Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Started work on histogram downsampling Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Removed compactor test Started work on histogram downsampling Naming Naming Raw downsampling without generics Rebase fix Refactor tests fragments into functions Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Refactor raw downsampling to support int histograms Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Comparing histograms for raw downsampling Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Add histogram support for bucket populateChunk Refactored RawDownsample code Benchmarking recovering series from histogram chunks Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Refactoring helper function into its own package Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Trying to implement integration test with querier and downsampled histogram chunks Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Making the test case a valid one Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Improvement for native histograms in bucket and series Extending downsampling test Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Fixing iteration over native histograms aggr chunks Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Added hist sum to aggrsFromFunc and populateChunk fix Adjusting query; Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Adding range as test parameter Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Fixing downsampling tests, general cleanup Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com> Fixed downsampling and improved test Fixed native histogram aggregate downsampling Fixed native histogram aggregate downsampling Improved native histogram downsampling Fixed downsample Improved native histogram downsampling Fixed reset hints for downsampling and added counter reset tests Added nore native histogram downsampling tests Improved native histograms downsampling tests. Fixed tests Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Added float histogram encoding for storepb Fixed linter Fixed lint Fixed lint Revert CHANGELOG.md Added debug log for downsampling Take schema into account when adding histograms Simplified downsampling (thanos-io#117) * Simplified downsampling Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Simplified downsampling Simplified downsampling * Fixed downsampling * Removed debug log * Fixed schema for downsampling native histograms * Fixed aggregate downsampling * Added fix for downsampling stale histograms * Lint fix for histogram downsampling tests * Added more stale marker checks for downsampling * Schema check for downsampled batch Added gauge histogram support for downsampling Adjust histogram sum downsampling for reset and hist AverageChunkIterator support More histogram tests and cleanup Added native histogram support to chunk iterator Fixed overlappingMerger emtpy fn Fixed overlappingMerger emtpy fn Fix for appending during compaction Disabled mimir unit tests Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixed imports Reverted querier changes Deleted native histogram query test Deleted native histogram query test Deleted native histogram query test Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixes from review Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Removed logger from downsampleaggr * Fixed populateChunk
danielmellado
pushed a commit
to danielmellado/thanos
that referenced
this pull request
Oct 16, 2023
OCPBUGS-12663: go.mod: update golang.org/x/net to v0.7.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Caching will be in next PR.