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-tee: ignore series count mismatches if all samples are within recent sample window #8749

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Jul 17, 2024

What this PR does

This PR fixes an issue where query-tee considers two responses to be different if they have different numbers of series, but all of the samples are within the recent sample window and so would be ignored during comparison.

For example, imagine it is now T=10, and the recent sample window is 2. Backend A returns a vector with 11 series, all with samples at T=10, and backend B returns 12 series, all with samples at T=10. (This can happen if the two queries are racing with writes for the queried data.)

Without this change, query-tee would consider the comparison failed, but if both responses had the same number of series, it would consider the comparison successful regardless of the sample values.

With this change, query-tee will consider both responses as equivalent.

Which issue(s) this PR fixes or relates to

(none)

Checklist

  • Tests updated.
  • [n/a] Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [n/a] about-versioning.md updated with experimental features.

…returned for a vector and all samples fall in the recent sample window
…returned for a matrix and all samples fall in the recent sample window
@charleskorn charleskorn marked this pull request as ready for review July 17, 2024 06:56
@charleskorn charleskorn requested a review from a team as a code owner July 17, 2024 06:56
@charleskorn charleskorn enabled auto-merge (squash) July 17, 2024 07:03
@krajorama krajorama self-requested a review July 17, 2024 07:40
Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Approved with some wording suggestions, the logic LGTM

tools/querytee/response_comparator.go Show resolved Hide resolved
tools/querytee/response_comparator_test.go Show resolved Hide resolved
tools/querytee/response_comparator_test.go Show resolved Hide resolved
@charleskorn charleskorn merged commit 176f71c into main Jul 17, 2024
29 checks passed
@charleskorn charleskorn deleted the charleskorn/query-tee-ignore-series-within-recent-sample-window branch July 17, 2024 08:01
charleskorn added a commit that referenced this pull request Jul 24, 2024
charleskorn added a commit that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants