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

Request retries for subgraph queries #2006

Merged
merged 11 commits into from
Nov 24, 2022
Merged

Request retries for subgraph queries #2006

merged 11 commits into from
Nov 24, 2022

Conversation

Geal
Copy link
Contributor

@Geal Geal commented Oct 26, 2022

Fix: #338
Fix: #1956

@github-actions

This comment has been minimized.

@Geal Geal changed the title WIP: retries for subgraph queries Request retries for subgraph queries Nov 17, 2022
@Geal Geal marked this pull request as ready for review November 17, 2022 17:17
@Geal
Copy link
Contributor Author

Geal commented Nov 17, 2022

I added some configuration options, marked as experimental, so we can move forward on this and at least have some form of retries, before we settle on the policy configuration format.

Copy link
Contributor

@o0Ignition0o o0Ignition0o left a comment

Choose a reason for hiding this comment

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

Can we open a followup issue on documentation? I think I have an intuition on how to customize it, but it s all still fuzzy to me

Copy link
Contributor

@BrynCooke BrynCooke left a comment

Choose a reason for hiding this comment

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

Approved, but I do think documentation should be added as part of this PR.

@Geal
Copy link
Contributor Author

Geal commented Nov 23, 2022

did we decide where documentation should be written for experimental features?

@bnjjj
Copy link
Contributor

bnjjj commented Nov 23, 2022

@Geal On my side for logging and tracing I wrote the documentation in the related section. For tracing in tracing.mdx for example. And just mark it as experimental

@bnjjj
Copy link
Contributor

bnjjj commented Nov 23, 2022

@Geal cf 6dc1333

@bnjjj bnjjj added the experimental Related to our experimental features/configurations label Nov 23, 2022
@Geal
Copy link
Contributor Author

Geal commented Nov 24, 2022

alright, there's docs now. I feel like the traffic shaping doc could benefit from longer explanations of each feature, and maybe a description of their order and interaction

@Geal
Copy link
Contributor Author

Geal commented Nov 24, 2022

also, as a follow-up, I think I'll add an option on retries, to deactivate them on mutations if necessary (not all mutations are idempotent)

@Geal Geal enabled auto-merge (squash) November 24, 2022 14:57
@Geal Geal merged commit 474ac76 into dev Nov 24, 2022
@Geal Geal deleted the geal/retries branch November 24, 2022 15:13
Geal added a commit that referenced this pull request Nov 28, 2022
garypen pushed a commit that referenced this pull request Nov 30, 2022
implements retries for subgraph requests. This uses Finagle's retry buckets algorithm
garypen pushed a commit that referenced this pull request Nov 30, 2022
@BrynCooke BrynCooke added this to the v1.5.0 milestone Dec 3, 2022
@garypen garypen added this to the v1.5.0 milestone Dec 5, 2022
@BrynCooke BrynCooke modified the milestone: v1.5.0 Dec 5, 2022
@BrynCooke BrynCooke mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Related to our experimental features/configurations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP2 error under load. subgraph retry policy and circuit breaking
5 participants