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

query: add partition labels flag #7722

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

MichaHoffmann
Copy link
Contributor

@MichaHoffmann MichaHoffmann commented Sep 11, 2024

The distributed engine decides when to push down certain operations by checking if the external labels are still present, i.e. we can push down a binary operation if its vector matching includes all external labels. This is great but if you have multiple external labels that are irrelevant for the partition this is problematic since query authors must be aware of those irrelevant labels and must incorporate them into their queries.
This PR attempts to solve that by giving an option to focus on the labels that are relevant for the partition.

Example: if you have region and datacenter external labels and every datacenter has a distinct label already then you could strip region for the decision if we can push something down. For example A * on (datacenter) B would be able to be pushed down but natively the engine would require it to be A * on (region, datacenter) B which is redundant somewhat.

An alternative that doesnt need new code would be to register the unnecessary labels as replica labels but it feels wrong since they dont correspond to any replicas and "region" is a horrible replica label.

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

Changes

  • Added a repeatable --query.partition-label flag

Verification

  • Added a unittest

The distributed engine decides when to push down certain operations by
checking if the external labels are still present, i.e. we can push down
a binary operation if its vector matching includes all external labels.
This is great but if you have multiple external labels that are
irrelevant for the partition this is problematic since query authors
must be aware of those irrelevant labels and must incorporate them into
their queries.
This PR attempts to solve that by giving an option to focus on the
labels that are relevant for the partition.

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
@MichaHoffmann MichaHoffmann force-pushed the mhoffmann/add-partition-labels-flag branch from 65ff47f to 19e384b Compare September 11, 2024 16:42
cmd/thanos/query.go Outdated Show resolved Hide resolved
Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
@MichaHoffmann MichaHoffmann force-pushed the mhoffmann/add-partition-labels-flag branch from 9746716 to e917201 Compare September 12, 2024 13:57
@MichaHoffmann MichaHoffmann merged commit d289189 into main Sep 12, 2024
20 of 22 checks passed
jnyi pushed a commit to jnyi/thanos that referenced this pull request Oct 17, 2024
* query: add partition labels flag

The distributed engine decides when to push down certain operations by
checking if the external labels are still present, i.e. we can push down
a binary operation if its vector matching includes all external labels.
This is great but if you have multiple external labels that are
irrelevant for the partition this is problematic since query authors
must be aware of those irrelevant labels and must incorporate them into
their queries.
This PR attempts to solve that by giving an option to focus on the
labels that are relevant for the partition.

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>

* Update cmd/thanos/query.go

Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>

---------

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com>
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.

2 participants