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

execution: partial responses in distributed engine #480

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MichaHoffmann
Copy link
Contributor

For very distributed setups we need to be able to deal with partial failures. This commit adds an option to continue evaulation if we encounter an error in a remote engine but dont want to fail the whole query.

@MichaHoffmann MichaHoffmann force-pushed the mhoffmann-handle-partial-responses-in-remote-engines branch 2 times, most recently from 2f5c05e to 4fbc06f Compare August 12, 2024 12:36
Copy link
Contributor

@harry671003 harry671003 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. Overall the changes looks good.
The lint is failing for a minor issue.

For very distributed setups we need to be able to deal with partial
failures. This commit adds an option to continue evaulation if we
encounter an error in a remote engine but dont want to fail the whole
query.

Signed-off-by: Michael Hoffmann <[email protected]>
@MichaHoffmann MichaHoffmann force-pushed the mhoffmann-handle-partial-responses-in-remote-engines branch from 4fbc06f to fbc6e69 Compare August 14, 2024 19:05
@fpetkovski
Copy link
Collaborator

I am not sure if this belongs in the engine to be fair. We already have this feature in Thanos so we could convert error responses to warnings when executing remote queries: https://github.com/thanos-io/thanos/blob/main/pkg/query/remote_engine.go#L231

@MichaHoffmann
Copy link
Contributor Author

I am not sure if this belongs in the engine to be fair. We already have this feature in Thanos so we could convert error responses to warnings when executing remote queries: https://github.com/thanos-io/thanos/blob/main/pkg/query/remote_engine.go#L231

Im kinda going back and forth about this in my head, but currently I feel like because the query engine orchestrates all the requests, it is the right place to make decisions about partial responses too. But this is mostly vibe based, just feels better to me right now.

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