0.4.x #189
0.4.x #189
Conversation
|
Changes Unknown when pulling fd40fc3 on kailuowang:0.4.x into ** on iheartradio:0.4.x**. |
|
@nsauro @lvauthrin @joprice anyone interested in reviewing this simple PR? |
| @@ -118,14 +118,20 @@ object Regulator { | |||
| weightOfLatestMetric: Double = 0.5 | |||
| ) | |||
|
|
|||
| private val MaximumEstimatedDelayRatio = 100000 | |||
There was a problem hiding this comment.
This is called ratio but isn't a fraction or ratio. Should this be a factor?
| ): Option[FiniteDuration] = { | ||
| val totalMillis = queueLength.value.toDouble / speed.value | ||
| //avoid creating too long an estimated delay, which is meaningless anyway | ||
| if (speed.value == 0 || totalMillis > (referenceDelay.toMillis * MaximumEstimatedDelayRatio)) None else Some(totalMillis.milliseconds) |
There was a problem hiding this comment.
Can we add a test for this? Also, do we just want to set an upper bound for the value or return None here? Not sure if one makes more sense.
There was a problem hiding this comment.
the None value is indicating that delay is too long to estimate (note it's None when the speed is zero) . I thought about caping it at the max, but that might give some false impression on grafana chart.
I am adding a test.
|
Changes Unknown when pulling f68fa52 on kailuowang:0.4.x into ** on iheartradio:0.4.x**. |
update gatling chart
|
Changes Unknown when pulling d5c85a1 on kailuowang:0.4.x into ** on iheartradio:0.4.x**. |
| @@ -118,14 +118,20 @@ object Regulator { | |||
| weightOfLatestMetric: Double = 0.5 | |||
| ) | |||
|
|
|||
| private val MaximumEstimatedDelayFactor = 100000 | |||
There was a problem hiding this comment.
I would jus prefix this with MS or something to denote the time unit
There was a problem hiding this comment.
This is a factor, not bound to a time unit. If the referenceDelay is 1 second the maximumEstimatedDelay would be referenceDelay * MaximumEstimatedDelayFactor = 1000000 second.
| @@ -52,7 +52,7 @@ class Regulator(settings: Settings, metricsCollector: ActorRef, regulatee: Actor | |||
| def receive: Receive = { | |||
| case s: Sample ⇒ | |||
| context become regulating(Status( | |||
| delay = estimateDelay(s, s.speed), | |||
| delay = estimateDelay(s, s.speed, settings.referenceDelay), | |||
There was a problem hiding this comment.
Is there no reference.conf update needed for this new value?
There was a problem hiding this comment.
This is not a new value, it's an existing configuration value.
No description provided.