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

Document ConcurrencyLimiter decisions #453

Open
iamdanfox opened this issue Feb 28, 2020 · 0 comments
Open

Document ConcurrencyLimiter decisions #453

iamdanfox opened this issue Feb 28, 2020 · 0 comments

Comments

@iamdanfox
Copy link
Contributor

Seems like dialogue makes a few changes from the 'battle-tested' behaviour that is running in prod right now, and I'd like to capture the reasons why somewhere.

  1. c-j-r waited for the request body to close before releasing a permit, dialogue just releases when the request comes back
  2. c-j-r kept a limiter per <pathTemplate,method> (which is roughly an endpoint), dialogue has one limiter per uri
  3. c-j-r used a forked ConjureWindowedLimit, dialogue uses a vanilla netflix WindowedLimit
  4. c-j-r considers 429 and 503 as failures (and 308 is a no-op), dialogue only counts 429 as a failure, all other response codes increase permits (!?). client-side exceptions are no-op.
  5. c-j-r uses a ThreadWorkQueue to ensure that one thread making a ton of requests can't starve another thread that just needs to make a few. Dialogue used to have a single queue, but currently just schedules retries on a single scheduled executor

For (1), I think the rationale is that way too many people fail to close all their response bodies, leading to degenerate behaviour where they time out waiting for concurrency limiters. Seems like we get 80% of the benefits of concurrency limiters without this extra strictness, and also protect ourselves from a ton of possible downsides due to misuse.

(4) just seems like a mistake?

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

No branches or pull requests

1 participant