Store: fix block dedup #6697
Merged
Store: fix block dedup #6697
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
MichaHoffmann
force-pushed
the
mhoffm-fix-store-block-dedup
branch
3 times, most recently
from
079539a
to
691d863
Compare
MichaHoffmann
commented
Sep 4, 2023
MichaHoffmann
force-pushed
the
mhoffm-fix-store-block-dedup
branch
from
691d863
to
3f9e2ae
Compare
MichaHoffmann
force-pushed
the
mhoffm-fix-store-block-dedup
branch
from
3f9e2ae
to
68140e4
Compare
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
MichaHoffmann
force-pushed
the
mhoffm-fix-store-block-dedup
branch
from
68140e4
to
dc87ac1
Compare
saswatamcode
approved these changes
Sep 4, 2023
2 tasks
yeya24
reviewed
Sep 5, 2023
| // If we have inner replica labels we need to resort. | ||
| s.mtx.Lock() | ||
| needsEagerRetrival := len(req.WithoutReplicaLabels) > 0 && s.labelNamesSet.HasAny(req.WithoutReplicaLabels) | ||
| s.mtx.Unlock() |
There was a problem hiding this comment.
Do we modify WithoutReplicaLabels somewhere so we need to lock?
There was a problem hiding this comment.
Label names set is updated concurrently
yeya24
approved these changes
Sep 5, 2023
matej-g
approved these changes
Sep 5, 2023
coleenquadros
pushed a commit
to coleenquadros/thanos
that referenced
this pull request
Sep 18, 2023
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
saswatamcode
pushed a commit
to saswatamcode/thanos
that referenced
this pull request
Sep 20, 2023
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
saswatamcode
added a commit
that referenced
this pull request
Sep 20, 2023
* Update thanos engine and Prometheus dependencies (#6664) * Update thanos engine and Prometheus dependencies This commit bumps thanos/promql-engine to latest main and resolves breaking changes from the prometheus/prometheus dependency. Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> * Add changelog entry Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> * Avoid closing head more than once Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> * Remove call to t.TempDir() Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> --------- Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> * build(deps): bump github.com/prometheus/alertmanager (#6671) Bumps [github.com/prometheus/alertmanager](https://github.com/prometheus/alertmanager) from 0.25.0 to 0.25.1. - [Release notes](https://github.com/prometheus/alertmanager/releases) - [Changelog](https://github.com/prometheus/alertmanager/blob/v0.25.1/CHANGELOG.md) - [Commits](prometheus/alertmanager@v0.25.0...v0.25.1) --- updated-dependencies: - dependency-name: github.com/prometheus/alertmanager dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * store: add acceptance tests for label methods to bucket store (#6668) Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Ruler: Add update label names routine for stateful ruler (#6689) Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Store: add some acceptance tests for label matching (#6691) Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Store: fix regex matching with set that matches empty (#6692) Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Store: add failing test for potential dedup issue (#6693) Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Store: fix block dedup (#6697) Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Add Shipper bytes uploaded metric #6438 (#6544) * [FEAT] Add uploaded bytes metric Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Add PR number to log Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FIX] Log msg Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Clean code Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FIX] Remove shadow code Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FIX] Go format Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Update objstore Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Update objstore package Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Update storage.md Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Update erroring bucket Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * [FEAT] Update erroring bucket Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> --------- Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> * Update objstore library to latest main (#6722) This commit updates the obstore library to the latest main version which optimizes the Iter operation to only request object names. Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> * Store: store responses should always be sorted (#6706) * Store: always sort, just compare labelset in proxy heap Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Store: add escape hatch to skip store resorting Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Store: remove stringset This is the wrong approach to detect if we need to resort. It cannot detect if we might end up with an unsorted series set if we add extLabels. Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> * Docs: drop paragraph about deduplication on inner labels Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> --------- Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> Co-authored-by: Michael Hoffmann <michael.hoffmann@aiven.io> * Updates busybox SHA (#6724) Signed-off-by: GitHub <noreply@github.com> Co-authored-by: fpetkovski <fpetkovski@users.noreply.github.com> * Cut patch release v0.32.3 Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Michael Hoffmann <mhoffm@posteo.de> Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> Signed-off-by: rita.canavarro <rita.canavarro@farfetch.com> Signed-off-by: GitHub <noreply@github.com> Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Hoffmann <mhoffm@posteo.de> Co-authored-by: Rita Canavarro <98762287+ritaCanavarro@users.noreply.github.com> Co-authored-by: Michael Hoffmann <michael.hoffmann@aiven.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: fpetkovski <fpetkovski@users.noreply.github.com>
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.
Should fix #6677
Changes
In case that we have inner replica labels in multiple blocks we need to resort before deduplicating.
Verification
Unittests