Skip to content

fault filter: reset token bucket on data start#6627

Merged
mattklein123 merged 7 commits intomasterfrom
fix_token_bucket
Apr 18, 2019
Merged

fault filter: reset token bucket on data start#6627
mattklein123 merged 7 commits intomasterfrom
fix_token_bucket

Conversation

@mattklein123
Copy link
Member

@mattklein123 mattklein123 commented Apr 17, 2019

This change alters the behavior of fault data limiting
by resetting the token bucket to a single token when data
initially starts streaming. This makes sure that data pacing
is as expected, while still allowing per-second bursting if
the data provider is also bursty.

Risk Level: Low
Testing: New and altered existing tests
Docs Changes: N/A
Release Notes: N/A

This change alters the behavior of fault data limiting
by resetting the token bucket to a single token when data
initially starts streaming. This makes sure that data pacing
is as expecting, while still allowing per-second bursting if
the data provider is also bursty.

Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

@ryancox PTAL

ryancox
ryancox previously approved these changes Apr 17, 2019
Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

@junr03 PTAL

Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

a couple comments

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

@junr03 updated. @junr03 @zuercher I updated the pedantic check script so that when run as a hook it doesn't check the extensions we don't support. If you want me to split it out into a different PR I can but it seems like not a big deal to leave here.

Signed-off-by: Matt Klein <mklein@lyft.com>
junr03
junr03 previously approved these changes Apr 18, 2019
Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

Up to you re: the pendantic script. My preference would be to have two different PRs to have explicit history. But if that is too obnoxious, then it is probably ok.

@mattklein123
Copy link
Member Author

@junr03 I will split out as I realize what I just did is broken. Will open a separate PR.

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

@junr03 reverted if you want to re-approve.

@mattklein123 mattklein123 merged commit 3b8e176 into master Apr 18, 2019
@mattklein123 mattklein123 deleted the fix_token_bucket branch April 18, 2019 21:57
mpuncel added a commit to mpuncel/envoy that referenced this pull request Apr 19, 2019
* master: (26 commits)
  docs: update docs to recommend /retest repokitteh command (envoyproxy#6655)
  http timeout integration test: wait for 15s for upstream reset (envoyproxy#6646)
  access log: add response code details to the access log formatter (envoyproxy#6626)
  build: add ppc build badge to README (envoyproxy#6629)
  Revert dispatcher stats (envoyproxy#6649)
  Batch implementation with timer (envoyproxy#6452)
  fault filter: reset token bucket on data start (envoyproxy#6627)
  event: update libevent dependency to fix race condition (envoyproxy#6637)
  examples: standardize docker-compose version and yaml extension (envoyproxy#6613)
  quiche: Implement SpdyUnsafeArena using SpdySimpleArena (envoyproxy#6612)
  router: support customizable retry back-off intervals (envoyproxy#6568)
  api: create OpenRCA service proto file (envoyproxy#6497)
  ext_authz: option for clearing route cache of authorized requests (envoyproxy#6503)
  build: update jinja to 2.10.1. (envoyproxy#6623)
  tools: check spelling in pre-push hook (envoyproxy#6631)
  security: blameless postmortem template. (envoyproxy#6553)
  Implementing Endpoint lease for ClusterLoadAssigment (envoyproxy#6477)
  add HTTP integration tests exercising timeouts (envoyproxy#6621)
  event: fix DispatcherImplTest::InitializeStats flake (envoyproxy#6619)
  Add tag extractor for RDS route config name (envoyproxy#6618)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
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.

3 participants