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

adaptive concurrency: Fix min_concurrency behavior when gradient shrinks #9947

Merged
merged 9 commits into from
Feb 7, 2020

Conversation

tonya11en
Copy link
Member

The min_concurrency setting dictated the concurrency limit when in a
min_rtt calculation window. However, if the gradient caused the
concurrency limit to shrink (due to increased latency), it was possible
for the limit to drop below this configured minimum. This patch fixes
this behavior so that the calculated concurrency limit is always >= the
configured minimum.

In addition, a minor change to the stat indicating whether the min_rtt
calculation window is active. When active, the stat was being set to the
concurrency limit; however, according to the documentation this should
be either 0 or 1. This patch also fixes this behavior.

Risk Level: Low, this filter is still experimental.
Testing: Adaptive concurrency tests

The min_concurrency setting dicated the concurrency limit when in a
min_rtt calculation window. However, if the gradient caused the
concurrency limit to shrink, it was possible for the limit to drop below
this configured minimum. This patch fixes this behavior so that the
calculated concurrency limit is always >= the configured minimum.

In addition, a minor change to the stat indicating whether the min_rtt
calculation window is active. When active, the stat was being set to the
concurrency limit; however, according to the documentation this should
be either 0 or 1. This patch also fixes this behavior.

Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
@mattklein123 mattklein123 self-assigned this Feb 6, 2020
@mattklein123
Copy link
Member

LGTM thanks. Can you merge master?

/wait

Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Signed-off-by: Tony Allen <tony@allen.gg>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit 1f7a342 into envoyproxy:master Feb 7, 2020
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