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

capacity-limiter-api: observer all gradient limit changes #3107

Conversation

bryce-anderson
Copy link
Contributor

Motivation:

We currently see callbacks to Observer.onLimitChange(..) only for the successful case, but it's perhaps even more important for the limit decrease case.

Modifications:

  • Call the callbacks whenever the limit changes.
  • Also call them on the failure case. Since we don't use the gradient and RTT params, we pass in -1.0. This also requires a change in the javadoc.
  • Add some tests to make sure the observers are called.

Motivation:

We currently see callbacks to Observer.onLimitChange(..) only for
the successful case, but it's perhaps even more important for the
limit decrease case.

Modifications:

- Call the callbacks whenever the limit changes.
- Also call them on the failure case. Since we don't use the
  gradient and RTT params, we pass in -1.0. This also requires
  a change in the javadoc.
- Add some tests to make sure the observers are called.
@bryce-anderson bryce-anderson changed the title capacity-limiter-api: Observer all gradient limit changes capacity-limiter-api: observer all gradient limit changes Nov 14, 2024
@bryce-anderson bryce-anderson merged commit 057cc56 into apple:main Nov 15, 2024
11 checks passed
@bryce-anderson bryce-anderson deleted the bl_anderson/GradientObserverLimitChanges branch November 15, 2024 17:18
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.

2 participants