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

rate limit: Add quota API #16942

Merged
merged 2 commits into from
Jun 15, 2021
Merged

rate limit: Add quota API #16942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90df5e9
Add rate limit quota API.
yanavlasov Jun 8, 2021
0a23d76
Address comments
yanavlasov Jun 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions api/envoy/service/ratelimit/v3/rls.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import "envoy/extensions/common/ratelimit/v3/ratelimit.proto";

import "google/protobuf/duration.proto";
import "google/protobuf/struct.proto";
import "google/protobuf/timestamp.proto";

import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.service.ratelimit.v3";
option java_outer_classname = "RlsProto";
Expand Down Expand Up @@ -101,6 +103,20 @@ message RateLimitResponse {
Unit unit = 2;
}

// Cacheable quota for responses, see documentation for the :ref:`quota
// <envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.DescriptorStatus.quota>` field.
// [#not-implemented-hide:]
message Quota {
// Number of matching requests granted in quota. Must be 1 or more.
uint32 requests = 1 [(validate.rules).uint32 = {gt: 0}];

oneof expiration_specifier {
// Point in time at which the quota expires.
google.protobuf.Timestamp valid_until = 2;
}
}

// [#next-free-field: 6]
message DescriptorStatus {
option (udpa.annotations.versioning).previous_message_type =
"envoy.service.ratelimit.v2.RateLimitResponse.DescriptorStatus";
Expand All @@ -116,6 +132,39 @@ message RateLimitResponse {

// Duration until reset of the current limit window.
google.protobuf.Duration duration_until_reset = 4;

// Quota granted for the descriptor. This is a certain number of requests over a period of time.
// The client may cache this result and apply the effective RateLimitResponse to future matching
// requests containing a matching descriptor without querying rate limit service.
//
// Quota is available for a request if its descriptor set has cached quota available for all
// descriptors.
//
// If quota is available, a RLS request will not be made and the quota will be reduced by 1 for
// all matching descriptors.
//
// If there is not sufficient quota, there are three cases:
// 1. A cached entry exists for a RLS descriptor that is out-of-quota, but not expired.
// In this case, the request will be treated as OVER_LIMIT.
// 2. Some RLS descriptors have a cached entry that has valid quota but some RLS descriptors
// have no cached entry. This will trigger a new RLS request.
// When the result is returned, a single unit will be consumed from the quota for all
// matching descriptors.
// If the server did not provide a quota, such as the quota message is empty for some of
// the descriptors, then the request admission is determined by the
// :ref:`overall_code <envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.overall_code>`.
// 3. All RLS descriptors lack a cached entry, this will trigger a new RLS request,
// When the result is returned, a single unit will be consumed from the quota for all
// matching descriptors.
// If the server did not provide a quota, such as the quota message is empty for some of
// the descriptors, then the request admission is determined by the
// :ref:`overall_code <envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.overall_code>`.
//
// When quota expires due to timeout, a new RLS request will also be made.
Copy link
Contributor

Choose a reason for hiding this comment

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

Proactively, as soon as the existing quota expires? Or do we wait for the next request to hit that descriptor before we make another RLS request?

Copy link
Member

Choose a reason for hiding this comment

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

I think this came up on the last review and I think this is client dependent. The client could do either. Can we clarify this?

Copy link
Contributor

Choose a reason for hiding this comment

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

The client can certainly try to guess whether it makes sense to proactively re-request, but I don't think it will want to do that unconditionally, because that would mean that the client would never stop requesting quota for that descriptor, even if it never again receives any requests for that descriptor. I'm wondering if there will be cases where different descriptors have different traffic properties, and instead of the client having to come up with hueristics to tell them apart, maybe it would make sense for the RLS server to provide a directive?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that makes sense to me, though IMO we could add that functionality later as it would be backwards compatible (some type of "fetch hint" message), but I don't feel super strongly about it

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, this could be added later. @yanavlasov, I'll leave it up to you whether you think it makes sense to add this now or defer it. But in the interim, we should certainly document the expectation here, so that it's clear to client implementors.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if I fully follow the "never stop requesting" logic; I think the idea is that as long as you have some quota, then it's safe to have a client intelligently attempt to refresh. When quota is exhausted, you probably want to wait for a request to trigger the fetch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the doc with proactively querying rate limit server.
I think it is possible to make client refresh quota only if it was being consumed during last refresh interval, so that it does not re-request if had no traffic for a rate limit descriptor.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't actually talking about the case where the quota is exhausted; I was talking about the case where the quota is expired (i.e., it may not be exhausted but its valid_until time has passed). Although now that you mention it, I think this concern also exists in the quota exhaustion case.

The "never stop requesting" situation I was thinking of was that if the client proactively refreshes the quota based solely on the fact that the valid_until time is approaching (or the fact that the quota is nearing exhaustion), then it will always refresh, even if it stops seeing requests that hit that descriptor. To avoid this, I think clients will also need to record the last time quota from a given descriptor was used, and if it's been too long since then, the client should stop refreshing that quota.

The problem I see here is that there several knobs in this algorithm that need to be set somehow:

  • The amount of time prior to valid_until at which quota should be refreshed.
  • The amount of quota remaining (maybe expressed as a percentage of the original quota given) at which quota should be refreshed.
  • The amount of time since quota was last used (I'll call this "idle time") at which the client should stop refreshing the quota.

No matter what we set these knobs to, there will be some situations in which the behavior will be sub-optimal. For example, if we set the idle time to 5 minutes but the client is actually seeing requests for that descriptor every 6 minutes, then we'll always wind up blocking the data plane for the RLS request. And if the RLS server is sending quota with valid_until times lasting 1 minute, then the client will needlessly proactively refresh the quota 4 times that it will never actually use.

Given that these knobs will really control the efficiency of the system (with the primary goal of minimizing the number of times we have to block a data plane request on an RLS request and a secondary goal of minimizing the number of unnecessary quota refreshes), I'm wondering if we need to make some of this configurable via xDS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I think as we start building this out, we can add more knobs to control refresh behavior.

// The implementation may choose to preemptively query the rate limit server for more quota on or
// before expiration or before the available quota runs out.
// [#not-implemented-hide:]
Quota quota = 5;
}

// The overall response code which takes into account all of the descriptors that were passed
Expand Down
49 changes: 49 additions & 0 deletions generated_api_shadow/envoy/service/ratelimit/v3/rls.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.