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

Removed Querier duplicate labels checks & made sure store will be not blocked for older Queriers. #1636

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

bwplotka
Copy link
Member

@bwplotka bwplotka commented Oct 11, 2019

Current idea is to remove duplication for now as it's tricky to make it right and we aim to unblock v0.8.0. Ticket to consider it back: #1635

Changes:

  • Cleaned up the code a bit.
  • Ensured proper locks.
  • thanos_store_nodes_grpc_connections is not per store type and external labels. thanos_store_node_info is marked as deprecated.
  • Added (optional, but enabled by default) compatibility label to store GW Info method (@thanos_compatibility_store_type" = "store". ) with flag to disable it. Store only adds it if there is any labelset to advertise.
  • Disabled any strict deduplication in external labels detection; just warning!
  • More tests; compatibility tests for query pre 0.8.0 and new store GW.
  • Remove compatibility label if spotted.

Fixes #1632

Signed-off-by: Bartek Plotka [email protected]

@metalmatze
Copy link
Contributor

Seems reasonable for the time being. 👍

@bwplotka bwplotka changed the title Fixed Querier duplicate labels checks & made sure store will be not blocked for older Queriers. Removed Querier duplicate labels checks & made sure store will be not blocked for older Queriers. Oct 13, 2019
@bwplotka bwplotka marked this pull request as ready for review October 13, 2019 22:59
@bwplotka
Copy link
Member Author

bwplotka commented Oct 13, 2019

This should work. PTAL @metalmatze @brancz @GiedriusS @povilasv @jojohappy if you have time. It is quite big ):

I build quay.io/thanos/thanos:v0.8.1-test-d042b5b image with this.

… blocked for older Queriers.

Changes:
* Cleaned up the code a bit.
* Ensured proper locks.
* thanos_store_nodes_grpc_connections is not per store type and external labels. thanos_store_node_info is marked as deprecated.
* Added (optional, but enabled by default) compatibility label to store GW Info method with flag to disable it. Store only adds it if there is any labelset to advertise.
* Disabled any strict deduplication in external labels detection; just warning!
* More tests; compatibility tests for query pre 0.8.0 and new store GW.
* Remove compatibility label if spotted.

Fixes #1632

Signed-off-by: Bartek Plotka <[email protected]>
Copy link
Member

@jojohappy jojohappy left a comment

Choose a reason for hiding this comment

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

Amazing! LGTM! Just a nit.

// Remove in next minor release.
metricDesc2: prometheus.NewDesc(
"thanos_store_node_info",
"Deprecated, use thanos_store_node_info instead.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Deprecated, use thanos_store_node_info instead.",
"Deprecated, use thanos_store_nodes_grpc_connections instead.",

Copy link
Member Author

Choose a reason for hiding this comment

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

ups (:

@bwplotka bwplotka merged commit 8d24d8a into master Oct 14, 2019
@brancz brancz deleted the issue-1632 branch October 14, 2019 10:55
GiedriusS pushed a commit that referenced this pull request Oct 28, 2019
… blocked for older Queriers. (#1636)

Changes:
* Cleaned up the code a bit.
* Ensured proper locks.
* thanos_store_nodes_grpc_connections is not per store type and external labels. thanos_store_node_info is marked as deprecated.
* Added (optional, but enabled by default) compatibility label to store GW Info method with flag to disable it. Store only adds it if there is any labelset to advertise.
* Disabled any strict deduplication in external labels detection; just warning!
* More tests; compatibility tests for query pre 0.8.0 and new store GW.
* Remove compatibility label if spotted.

Fixes #1632

Signed-off-by: Bartek Plotka <[email protected]>
Signed-off-by: Giedrius Statkevičius <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants