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

Pass QueryEngineFactory by reference #6331

Merged
merged 2 commits into from
May 3, 2023

Conversation

rabenhorst
Copy link
Contributor

Using both the GRPC and HTTP query API of one querier results in duplicate metrics collector registration attempted. This happens because the queryEngineFactory is currently passed by value, so there will be two instances of the factory resulting in two engines being created when using both the GRPC and HTTP query API of the same querier. Since both engines will register the same metrics we get the duplication error. To fix this we change to passing the engine factory by reference in this PR.

@rabenhorst rabenhorst changed the title Pass engine factory by reference Pass QueryEngineFactory by reference May 3, 2023
pkg/api/query/v1_test.go Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/M and removed size/S labels May 3, 2023
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

DCO missing?

@rabenhorst
Copy link
Contributor Author

DCO missing?

Fixed

CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Cleanup

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
@fpetkovski fpetkovski enabled auto-merge May 3, 2023 13:56
@fpetkovski fpetkovski merged commit 1d08ebf into thanos-io:main May 3, 2023
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.

3 participants