Add compatibility for sort without ext labels #6451
Conversation
fpetkovski
force-pushed
the
fix-proxy-sort
branch
from
cdadaa4 to
90a7f3f
Compare
In thanos-io#6299 we changed the comparison algorithm in the store proxy to ignore store-specific external labels. This causes problems during long rollouts because old stores and new stores will sort data differently, leading to incorrectly deduplicated results. This commit adds a new flag to the store info message to indicate whether the store sorts data with or without external labels. The querier uses this flag to decide whether to resort the store response set before starting a merge. Signed-off-by: Filip Petkovski <[email protected]>
fpetkovski
force-pushed
the
fix-proxy-sort
branch
from
90a7f3f to
d71ea7f
Compare
Signed-off-by: Filip Petkovski <[email protected]>
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| for _, s := range set { | ||
| func sortWithoutLabels(set []*storepb.SeriesResponse, storeLabels map[string]struct{}, labelsToRemove map[string]struct{}) { | ||
| extLabels := make([]labels.Labels, len(set)) | ||
| lblScratch := labels.Labels{} |
There was a problem hiding this comment.
SA4006: this value of lblScratch is never used
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| }) | ||
|
|
||
| for i, _ := range set { |
There was a problem hiding this comment.
S1005: unnecessary assignment to the blank identifier
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| }) | ||
|
|
||
| for i, _ := range set { |
There was a problem hiding this comment.
gofmt: File is not gofmt-ed with -s
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| for _, s := range set { | ||
| func sortWithoutLabels(set []*storepb.SeriesResponse, storeLabels map[string]struct{}, labelsToRemove map[string]struct{}) { | ||
| extLabels := make([]labels.Labels, len(set)) | ||
| lblScratch := labels.Labels{} |
There was a problem hiding this comment.
ineffassign: ineffectual assignment to lblScratch
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
|
We will try to solve this problem differently. It seems hard to fix it with compatibility flags. |
In #6299 we changed the comparison algorithm in the store proxy to ignore store-specific external labels. This causes problems during long rollouts because old stores and new stores will sort data differently, leading to incorrectly deduplicated results.
This commit adds a new flag to the store info message to indicate whether the store sorts data with or without external labels. The querier uses this flag to decide whether to resort the store response set before starting a merge.
Changes
Verification