markusthoemmes commented on a change in pull request #3272: improve
loadbalancer's schedule algorithm
URL:
https://github.com/apache/incubator-openwhisk/pull/3272#discussion_r167427306
##########
File path:
core/controller/src/main/scala/whisk/core/loadBalancer/ContainerPoolBalancer.scala
##########
@@ -345,7 +345,8 @@ object ContainerPoolBalancer extends LoadBalancerProvider {
.find(_._3 < invokerBusyThreshold)
.orElse(invokerProgression.find(_._3 < invokerBusyThreshold * 2))
.orElse(invokerProgression.find(_._3 < invokerBusyThreshold * 3))
- .orElse(invokerProgression.headOption)
+ .orElse(
+ if (invokerProgression.size == 0) None else
invokerProgression.lift(Random.nextInt(invokerProgression.size)))
Review comment:
It is **crucial** to use `ThreadLocalRandom` vs. `Random` because getting
randomness is synchronized and thus will result in thread-contention.
For good measure, please use
```scala
if(invokerProgression.nonEmpty)
Some(invokerProgression(ThreadLocalRandom.current.nextInt(invokerProgression.size)))
else None
```
I wouldn't use `lift` (and haven't in my implementation of it) because any
error at that case is really a bug and should be surfaced as such.
The use of `nonEmpty`/`isEmpty` is generally encouraged over a size-check.
More readable and more performant if you have a lazy data-structure at hand
(not relevant here, since you need the size anyway but a good thing to become
accustomed to)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
