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.

Sign up for GitHub

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

Jump to bottom

Introduce traffic resiliency features #2911

Merged
merged 4 commits into from
May 8, 2024
Merged

Introduce traffic resiliency features #2911

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
bryce-anderson
bryce-anderson reviewed May 8, 2024
View reviewed changes
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.

servicetalk-capacity-limiter-api/build.gradle Outdated Show resolved Hide resolved
...ter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/GradientCapacityLimiter.java Show resolved Hide resolved
...er-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/CompositeCapacityLimiter.java Outdated Show resolved Hide resolved
...api/src/main/java/io/servicetalk/apple/capacity/limiter/api/FixedCapacityLimiterBuilder.java Outdated Show resolved Hide resolved
...ter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/GradientCapacityLimiter.java Outdated Show resolved Hide resolved
.../src/main/java/io/servicetalk/apple/capacity/limiter/api/GradientCapacityLimiterBuilder.java Outdated Show resolved Hide resolved
...acity-limiter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/Preconditions.java Outdated Show resolved Hide resolved
bryce-anderson
bryce-anderson approved these changes May 8, 2024
View reviewed changes
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.

...city-limiter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/Classification.java Outdated Show resolved Hide resolved
@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
idelpivnitskiy
idelpivnitskiy reviewed May 8, 2024
View reviewed changes
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:

gradle.properties Show resolved Hide resolved
servicetalk-utils-internal/src/main/java/io/servicetalk/utils/internal/NumberUtils.java Show resolved Hide resolved
...limiter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/AimdCapacityLimiter.java Show resolved Hide resolved
...ity-limiter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/CapacityLimiter.java Show resolved Hide resolved
...ter-api/src/main/java/io/servicetalk/apple/capacity/limiter/api/AllowAllCapacityLimiter.java Show resolved Hide resolved
...in/java/io/servicetalk/apple/traffic/resilience/http/TrafficResilienceHttpServiceFilter.java Show resolved Hide resolved
...in/java/io/servicetalk/apple/traffic/resilience/http/TrafficResilienceHttpServiceFilter.java Show resolved Hide resolved
...in/java/io/servicetalk/apple/traffic/resilience/http/TrafficResilienceHttpServiceFilter.java Show resolved Hide resolved
.../src/main/java/io/servicetalk/apple/traffic/resilience/http/PeerCapacityRejectionPolicy.java Show resolved Hide resolved
servicetalk-capacity-limiter-api/build.gradle Show resolved Hide resolved
@idelpivnitskiy idelpivnitskiy mentioned this pull request May 9, 2024
Merged
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request May 9, 2024
@idelpivnitskiy
Follow-up comments for traffic resilience features
674b4cc 
Motivation:

Minor comments for apple#2911.
@idelpivnitskiy idelpivnitskiy mentioned this pull request May 9, 2024
Merged
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request May 9, 2024
@idelpivnitskiy
Follow-up comments for traffic resilience features
ce6454f 
Motivation:

Minor comments for apple#2911.
idelpivnitskiy added a commit that referenced this pull request May 9, 2024
@idelpivnitskiy
Follow-up comments for traffic resilience features (#2915)
7bb8b6d 
Motivation:

Minor comments for #2911.
This was referenced May 10, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idelpivnitskiy idelpivnitskiy idelpivnitskiy left review comments

@bryce-anderson bryce-anderson bryce-anderson approved these changes

@daschl daschl Awaiting requested review from daschl

Assignees

@tkountis tkountis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tkountis @bryce-anderson @idelpivnitskiy