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-frontend: Parrallelize unmarshalling of responses #3713

Merged
merged 13 commits into from
Jun 4, 2024

Conversation

joe-elliott
Copy link
Member

What this PR does:

Adds a configurable number of dedicated goroutines per request to consume the responses and add them to the combiner. The combiner was also adjusted to move the unmarshalling steps outside of the locking logic so we could do multiple unmarshals in parrallel.

  • Added a parameter to configure the number of goroutines. Default: 10
  • Added tests to confirm the combiner doesn't race with the new changes
  • Found and fixed an issue where tag values v2 was not initializing its status code to 200 which caused it to fail if GRPCDiff was called before a response was received.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

modules/frontend/pipeline/collector_http.go Outdated Show resolved Hide resolved
modules/frontend/pipeline/collector_http.go Show resolved Hide resolved
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Thank you for updating the config doc.

Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
Copy link
Member Author

Fixed a flakey test and add http status code to the logged response on searches.

Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott joe-elliott merged commit 01f4bc8 into grafana:main Jun 4, 2024
15 checks passed
mapno pushed a commit that referenced this pull request Jun 6, 2024
* Add combiner does not race test

Signed-off-by: Joe Elliott <number101010@gmail.com>

* make unmarshal in combiner //able

Signed-off-by: Joe Elliott <number101010@gmail.com>

* wip: one test failing

Signed-off-by: Joe Elliott <number101010@gmail.com>

* store first err

Signed-off-by: Joe Elliott <number101010@gmail.com>

* config

Signed-off-by: Joe Elliott <number101010@gmail.com>

* lint

Signed-off-by: Joe Elliott <number101010@gmail.com>

* enhance

Signed-off-by: Joe Elliott <number101010@gmail.com>

* lint n' stuff

Signed-off-by: Joe Elliott <number101010@gmail.com>

* addNextAsync -> consumeAndCombineResponses

Signed-off-by: Joe Elliott <number101010@gmail.com>

* fix race

Signed-off-by: Joe Elliott <number101010@gmail.com>

* fixed test

Signed-off-by: Joe Elliott <number101010@gmail.com>

* fix race

Signed-off-by: Joe Elliott <number101010@gmail.com>

* test to confirm errors are held

Signed-off-by: Joe Elliott <number101010@gmail.com>

---------

Signed-off-by: Joe Elliott <number101010@gmail.com>
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.

3 participants