api
api copied to clipboard
first draft of rate limit API
But please not in MeshConfig
Rate limit feels like a similar category as telemetry,tracing,and ext authz backend. So why not follow the same pattern?
If we don't like that we should change everything IMO. It may be better to he consistently "wrong" than inconsistent?
@costinm
- All variations are possible:
- local rate limit on outbound route at ingress;
- global rate limit on outbound route at ingress;
- local rate limit on app sidecar
- global rate limit on app sidecar
- Usage of provider is for consistency with the rest of APIs (e.g. telemetry API). This was strongly requested AFAIR, and I don't want to have a special way just for rate limit. Let's not use this PR to resurrect that discussion and keep it separate.
Token buckets are standard for implementing rate limiting. The question I suspect is if they should be exposed as part of the API, and if users should be expected ( or allowed ) to mess with this.
On Tue, Jun 22, 2021 at 10:15 PM Kuat @.***> wrote:
@.**** commented on this pull request.
In policy/v1alpha1/ratelimit.proto https://github.com/istio/api/pull/2028#discussion_r656765466:
- // The number of tokens added to the bucket during each fill interval. If not specified, defaults
- // to a single token.
- google.protobuf.UInt32Value tokens_per_fill = 2;
- // The fill interval that tokens are added to the bucket. During each fill interval
- //
tokens_per_fillare added to the bucket. The bucket will never contain more than- //
max_tokenstokens. The interval must be >=50ms.- google.protobuf.Duration fill_interval = 3;
- }
- // The token bucket configuration to use for rate limiting requests that are
- // processed by this filter. Each request processed by the filter consumes a
- // single token. If the token is available, the request will be allowed. If
- // no tokens are available, the proxy responds with 429.
- TokenBucket token_bucket = 1;
Token buckets are a standard concept in rate limiting. See https://cloud.google.com/architecture/rate-limiting-strategies-techniques#techniques-enforcing-rate-limits, for example. Would wrapping this into "algorithm" proto with a sole field address the concern?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/istio/api/pull/2028#discussion_r656765466, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2XZPMBPNCY6BKPFVRTTUFUY3ANCNFSM464QUHZQ .
Token buckets are standard for implementing rate limiting. The question I suspect is if they should be exposed as part of the API, and if users should be expected ( or allowed ) to mess with this. … The alternative is that we have no istio api, but controllers for backends to program envoy. What this means is that if someone uses Google cloud armor to configure their rate limits we will use that config to correctly configure the proxy. There will be a similar rls controller that will read back end rls config and program envoy. These cotrollers may require istiod extensions or output an EnvoyFilter.
Managed grpc may or may not implement it, but are Likely to implement rls anyway.
The uses cases that customers need, rate limiting based on service account or based on presence and values of multiple headers cannot be expressed in a simpler api. But using the alternative above we can solve that problem for specific backend without adding an api.
a least common denominator istio api can be orthogonally added .
Right, I haven't heard a single use case that only needs service-service rate limit. The least common denominator API does not answer anyone's needs. You are right, this is just a principle EnvoyFilter for rate limiting. But that can be said about authz and others, they are just crafted subsets of xDS with better operational user experience. EnvoyFilter are incredibly error prone.
On Wed, Jun 23, 2021 at 12:14 AM mandarjog @.***> wrote:
Token buckets are standard for implementing rate limiting. The question I suspect is if they should be exposed as part of the API, and if users should be expected ( or allowed ) to mess with this. … <#m_5533683122270881071_> The alternative is that we have no istio api, but controllers for backends to program envoy. What this means is that if someone uses Google cloud armor to configure their rate limits we will use that config to correctly configure the proxy. There will be a similar rls controller that will read back end rls config and program envoy. These cotrollers may require istiod extensions or output an EnvoyFilter.
Managed grpc may or may not implement it, but are Likely to implement rls anyway.
The uses cases that customers need, rate limiting based on service account or based on presence and values of multiple headers cannot be expressed in a simpler api. But using the alternative above we can solve that problem for specific backend without adding an api.
a least common denominator istio api can be orthogonally added .
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/istio/api/pull/2028#issuecomment-866591645, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIYRRXH4U34EGTYLW3BVDLTUGCVLANCNFSM464QUHZQ .
It's ok to retry on 429/503 but the client needs to back-off. I think what John suggested is just a regular LB which does not take into account back pressure. gRPC throttles itself on client-side which is why it's considered "safe", although there've been some cascading failures due to rate limited retries.
On Mon, Jun 28, 2021 at 9:44 AM mandarjog @.***> wrote:
@.**** commented on this pull request.
In policy/v1alpha1/ratelimit.proto https://github.com/istio/api/pull/2028#discussion_r659948796:
+// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License.
+syntax = "proto3"; + +import "type/v1beta1/selector.proto"; +import "google/protobuf/wrappers.proto"; +import "google/protobuf/duration.proto"; + +option go_package = "istio.io/api/policy/v1alpha1"; + +package istio.policy.v1alpha1;
503 and 429 are similar, and envoy treats x-envoy-retry-grpc-on / resource exhausted as retryable.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/istio/api/pull/2028#discussion_r659948796, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIYRRVMMP5QHX2HNPXE7QDTVCRHNANCNFSM464QUHZQ .
@kyessenov: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.