platinummonkey / go-concurrency-limits

Go implementation of Netflix/concurrency-limits
Apache License 2.0
119 stars 21 forks source link

How to use average RRT instead of min RRT in DefaultListener #60

Closed mingkhuang closed 3 years ago

mingkhuang commented 3 years ago

Hi, I want to get a Limiter as below. What is the suitable way? Thanks! BTW, do you evaluate these algorithms? How about the results? What is your scenery to use these algorithms?

diff --git a/limiter/default.go b/limiter/default.go
index be0e629..6aaee13 100644
--- a/limiter/default.go
+++ b/limiter/default.go
@@ -73,7 +73,7 @@ func (l *DefaultListener) OnSuccess() {
                                l.limiter.nextUpdateTime = endTime + minVal
                                l.limiter.limit.OnSample(
                                        0,
-                                       current.CandidateRTTNanoseconds(),
+                                       current.AverageRTTNanoseconds(),
                                        current.MaxInFlight(),
                                        current.DidDrop(),
                                )
platinummonkey commented 3 years ago

๐Ÿ‘‹ it's an interface - I would suggest either:

  1. make a PR with a custom implementation or flag to set the options on the Default type of limiter && listener.
  2. Alternatively make an entirely new implementation. The Default is an easy-getting started implementation.

BTW, do you evaluate these algorithms? How about the results? What is your scenery to use these algorithms?

Effectively this is applying Little's Law + a few alternative algorithms for queueing theorems on how a dynamic system behaves and the appropriate stable conditions. Keep in mind while the library applies the given algorithms, it isn't prescriptive about how to determine the appropriate tune-ables, this will require careful measurement, planning and planning game-days to validate your assumptions and changes.

mingkhuang commented 3 years ago

For 1, do you mean adding an option to DefaultLimiter to control which SampleWindow to use? For 2, it leads to too much duplicated code, I think. For BTW, do you mean that these algorithms are just reference, we need to adjust them carefully for usage?

Thanks!

platinummonkey commented 3 years ago

For 1, do you mean adding an option to DefaultLimiter to control which SampleWindow to use?

Yes ๐Ÿ˜„

For 2, it leads to too much duplicated code, I think.

totally fair

For BTW, do you mean that these algorithms are just reference, we need to adjust them carefully for usage?

Right the algorithms are just logical implementations of tuning dynamic system feedback throttling. The hardest part is not the code, but tuning your parameters which requires some good observability and good SLO commitments from the downstream services this would be leveraging.

An example might be:

There are other strategies like the Vegas style which leverages the limit + measurement samples to dynamically adjust the throttling based on observed measurements of Service B. Which is useful if Service B begins to become degraded, you're effectively preventing thundering herd problems but not totally turning off all requests.

You might think of this as a dynamic adaptive circuit breaker.

mingkhuang commented 3 years ago

Previously, I thought adaptive algorithms need no tune, and they can free us from tedious tuning work. It seems I'm too naive. So, what is the advantage of adaptive algorithms compared to fixed threshold strategy? Adapt to changing environment?

Thanks๏ผ

platinummonkey commented 3 years ago

Consider this a level 2 type of tool on the quest to smarter ("level 3+") tooling ๐Ÿ˜† It makes some assumptions and needs some guidelines. Though that isn't to say another tool or automation couldn't introspect your current configurations and dynamic systems to auto-tune the behavior.

mingkhuang commented 3 years ago

Thanks! You teach me a lot. The issue could be closed.

platinummonkey commented 3 years ago

๐Ÿ‘