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

Introduce traffic resiliency features #2911

Merged
merged 4 commits into from
May 8, 2024

Conversation

tkountis
Copy link
Contributor

@tkountis tkountis commented May 7, 2024

Motivation

Lack of common resiliency features like breakers, limiters and integrations with the remaining of the client workflows.

Modifications

Support for

  • Circuit breakers
  • Capacity limiters (rate, quotas, dynamic, fixed)
    • Request clacification
  • Server feedback evaluation
    • Retry after
    • Limit adaptation

@tkountis tkountis self-assigned this May 7, 2024
@tkountis tkountis requested review from daschl and bryce-anderson May 7, 2024 20:50
@tkountis tkountis force-pushed the traffic-resiliency branch from 46eb23c to 8b1e23c Compare May 8, 2024 14:41
Copy link
Contributor

@bryce-anderson bryce-anderson left a comment

Choose a reason for hiding this comment

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

Looks good other than I'm curious if we can remove the deprecated API's.

@tkountis tkountis requested a review from bryce-anderson May 8, 2024 19:04
Copy link
Contributor

@bryce-anderson bryce-anderson left a comment

Choose a reason for hiding this comment

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

🚢 after fixing the weight() doc string. I'd personally prefer to change the method name to priority() since it aligns well with the documentation of it but I'm not strongly opinionated about it.

@idelpivnitskiy idelpivnitskiy self-requested a review May 8, 2024 20:10
@tkountis tkountis merged commit baac8d6 into apple:main May 8, 2024
15 checks passed
@tkountis tkountis deleted the traffic-resiliency branch May 8, 2024 21:07
Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

There is a patch in my last comment that addresses some other findings. Consider applying it first before working on other comments:

idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request May 9, 2024
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request May 9, 2024
idelpivnitskiy added a commit that referenced this pull request May 9, 2024
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