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

Store: add failing test for proxy heap sorting issue #6702

Conversation

mhoffm-aiven
Copy link
Contributor

This test was created with production data from a user sent to me. It shows that despite getting sorted responses from store apis the end result of proxy heap is not sorted because of adverse store labels. The offender in this case is that receivers are not instructed to drop the "cluster" label so all series from the receivers are sorted before series from the store making them overall unsorted in the prometheu sense.

Signed-off-by: Michael Hoffmann <michael.hoffmann@aiven.io>
@mhoffm-aiven
Copy link
Contributor Author

Store labels: image
Receive labels:
image

@MichaHoffmann
Copy link
Contributor

The test failure is intended to show the issue

@MichaHoffmann
Copy link
Contributor

So i have a proposition how to solve all this:

  • we should establish the invariant that store apis always respond with sorted series; to do that in first iteration we pipe all sends through "resortingServer" at first
  • if we do it like that we can get rid of cuckoofilter stuff since we are ( at first ) always resorting to keep the invariant
  • if we have the invariant that store responses are sorted we can just use whole labelset in proxy heap Less and end up sorted in the expected way always; thus solving the problem
  • we can optimize the resorting away in some cases in followups if we can prove that the response is already sorted ( or still sorted after adding extlabels )

@mhoffm-aiven
Copy link
Contributor Author

Continued in #6706

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