Feature/auto scaling with usage #47

lftakakura · 2018-08-28T22:46:33Z

No description provided.

coveralls · 2018-08-28T23:03:08Z

Coverage increased (+0.4%) to 82.017% when pulling 0a4d73b on feature/auto-scaling-with-usage into b67fa88 on master.

henrod

Good, just some changes

henrod · 2018-08-29T12:38:29Z

watcher/watcher.go

@@ -600,75 +588,208 @@ func (w *Watcher) checkState(
 	roomCount *models.RoomsStatusCount,
 	scheduler *models.Scheduler,
 	nowTimestamp int64,
-) (bool, bool, bool, error) { //shouldScaleUp, shouldScaleDown, changedState
+) (int, bool, bool, bool, error) { //delta, shouldScaleUp, shouldScaleDown, changedState


5 returns is too much, better create a new struct that contains the int and the three bools, then you return that struct and the error

can't scaleup/down be inferred from the delta sign? if positive we should scaleup and if negative we should scaledown

henrod · 2018-08-29T13:00:22Z

watcher/watcher.go

+
+		// If usage is above limit, scale up.
+		if isDown == false && 100*roomCount.Occupied >= roomCount.Total()*trigger.Limit {
+			w.Logger.Debug("Usage is above limit. Should scale up")


This can be extracted to another function. The idea is reduce the size of this function and simplify it

henrod · 2018-08-29T13:12:50Z

watcher/watcher.go

+	if 100*roomCount.Occupied >= roomCount.Total()*autoScalingInfo.Up.Trigger.Limit {
+		scaleInfo.SendUsage(
+			w.SchedulerName,
+			"legacy", point, roomCount.Total(),


Better to put "legacy" on constants to avoid typos

words of wisdom

cscatolini

Just a few comments

cscatolini · 2018-08-29T14:40:31Z

dev-room/app.py

@@ -37,6 +48,7 @@ def ping():
    except Exception as ex:
        print(str(ex))
        pass
+        break


why the pass and the break?

it shouldn't have break. Forgot to take it off =/

cscatolini · 2018-08-29T16:19:17Z

watcher/utils.go

+	roomCount *models.RoomsStatusCount,
+) int { // delta
+	usageThreshold := float32(metricTrigger.Usage) / 100
+	return int(float32(float32(roomCount.Occupied)-float32(usageThreshold*float32(roomCount.Total()))) / usageThreshold)


shouldn't we round up instead of just converting to int?

cscatolini · 2018-08-29T16:20:23Z

watcher/watcher.go

@@ -600,75 +588,208 @@ func (w *Watcher) checkState(
 	roomCount *models.RoomsStatusCount,
 	scheduler *models.Scheduler,
 	nowTimestamp int64,
-) (bool, bool, bool, error) { //shouldScaleUp, shouldScaleDown, changedState
+) (int, bool, bool, bool, error) { //delta, shouldScaleUp, shouldScaleDown, changedState


can't scaleup/down be inferred from the delta sign? if positive we should scaleup and if negative we should scaledown

cscatolini · 2018-08-29T16:22:56Z

watcher/watcher.go

+	} else if inSync == true { // always send current usage
+		if len(autoScalingInfo.Down.MetricsTrigger) > 0 {
+			for _, trigger := range autoScalingInfo.Down.MetricsTrigger {
+				w.ScaleDownInfo.SendUsage(


I couldn't find any sendUsage for scale up, it this intended? or am I missing something?

On scaleUp it will call SendUsageAndReturnStatus(). SendUsage() is called here because checkMetricsTrigger will not be called for down scaling if up scaling is needed.

so we have two separate usage metrics, one for scaling up and another for scaling down?

cscatolini · 2018-08-29T16:31:31Z

watcher/watcher.go

+	if 100*roomCount.Occupied >= roomCount.Total()*autoScalingInfo.Up.Trigger.Limit {
+		scaleInfo.SendUsage(
+			w.SchedulerName,
+			"legacy", point, roomCount.Total(),


words of wisdom

cscatolini

Will these changes be backwards compatible? I don't think so but I'm not sure it will be a big problem.

cscatolini · 2018-08-30T21:53:13Z

watcher/utils.go

@@ -30,6 +31,12 @@ func roomDynamicDelta(
 	metricTrigger *models.ScalingPolicyMetricsTrigger,
 	roomCount *models.RoomsStatusCount,
 ) int { // delta
-	usageThreshold := float32(metricTrigger.Usage) / 100
-	return int(float32(float32(roomCount.Occupied)-float32(usageThreshold*float32(roomCount.Total()))) / usageThreshold)
+	// [Occupied / (Total + Delta)] = Usage/100


much clearer 😌

lftakakura · 2018-08-31T13:09:51Z

The changes should be backwards compatible. The only issue is with the redis set that now is the same for both up and down scaling.

henrod · 2018-08-31T13:27:29Z

watcher/utils.go

+)
+
+func dynamicDelta(
+	metricTrigger *models.ScalingPolicyMetricsTrigger,


You could use ScalingPolicyMetricsTrigger as an interface actually, with the methods Metric() and Usage(). Or with the method DynamicDelta(). That way you won't need a switch/case statement.

henrod · 2018-08-31T13:28:43Z

watcher/watcher.go

-) (bool, bool, bool, error) { //shouldScaleUp, shouldScaleDown, changedState
-	var shouldScaleUp, shouldScaleDown, changedState bool
-	inSync := true
+) (*scaling, error) { //delta, shouldScaleUp, shouldScaleDown, changedState


We don't need this comment anymore ;)

henrod · 2018-08-31T13:31:07Z

watcher/watcher.go

-	usage := float32(autoScalingInfo.Up.Trigger.Usage) / 100
-	capacity := autoScalingInfo.Up.Trigger.Time / w.AutoScalingPeriod
+	// Send current usage
+	if len(autoScalingInfo.Down.MetricsTrigger) > 0 {


This method is too long. You could separate the ifs into methods whose names are their intention.

henrod · 2018-08-31T13:35:12Z

watcher/watcher.go

+	return scaling, nil
+}
+
+func (w *Watcher) checkLegacyTrigger(


There is too much duplicated code with checkMetricsTrigger. Couldn't you merge them? For example, The for loop on checkMetricsTrigger could be extracted and receive the same parameters checkLegacyTrigger uses.

…terminating status)

cscatolini · 2018-09-06T22:14:45Z

autoscaler/cpu_and_mem_scaler.go

+func getMemUsageAndRequests(usage, requests v1.ResourceList) (float32, float32) {
+	resourceRequests := float32(requests.Memory().ScaledValue(0))
+	resourceUsage := float32(usage.Memory().ScaledValue(0))
+	fmt.Printf("\nrequests: %v | usage: %v\n", requests.Memory().Value(), usage.Memory().Value())


debug logs?

cscatolini · 2018-09-06T22:16:56Z

autoscaler/cpu_and_mem_scaler.go

+	usageRatio := currentUtilization / float32(trigger.Usage) / 100
+	delta := int(math.Ceil(float64(usageRatio)*float64(roomCount.Available()))) - roomCount.Available()
+
+	fmt.Printf("\ncurrentUtilization: %v | usageRatio: %v | roomCount: %v | delta: %v\n", currentUtilization, usageRatio, roomCount.Available(), delta)


cscatolini · 2018-09-06T23:31:51Z

autoscaler/cpu_and_mem_scaler.go

+
+	for _, pod := range podList.Items {
+		for i, container := range pod.Containers {
+			if container.Name == pod.Name { // Get game room container


probably this is not the best way to check

henrod · 2018-09-10T12:44:52Z

api/app.go

@@ -56,6 +57,7 @@ type App struct {
 	DBClient         *pg.Client
 	RedisClient      *redis.Client
 	KubernetesClient kubernetes.Interface
+	KubernetesMetricsClient metricsClient.Interface


linter should align this

henrod · 2018-09-13T13:45:02Z

controller/controller_test.go

@@ -538,6 +644,98 @@ portRange:
 			})
 		})

+		FIt("should return error if autoscaling min > max", func() {


All tests must be It()

cscatolini · 2018-09-14T18:39:25Z

dev-room/api/increaseResourceUsage.py

@@ -0,0 +1,32 @@
+import falcon


@lftakakura this is the best file in the project 🤣

Luiz Felipe Takakura added 4 commits August 23, 2018 14:26

Fix kube svc creation with multiple containers per pod

b784cbd

Implement MetricsTrigger (WIP)

656e7d4

Add sigint and sigterm handler

cf0875c

Implement metrics scaling with room usage

751a6ca

lftakakura requested review from cscatolini and henrod August 28, 2018 22:46

henrod reviewed Aug 29, 2018

View reviewed changes

Implement more scale_info tests

6a3a646

cscatolini reviewed Aug 29, 2018

View reviewed changes

Refactor AutoScaele

367a108

cscatolini reviewed Aug 30, 2018

View reviewed changes

henrod suggested changes Aug 31, 2018

View reviewed changes

Luiz Felipe Takakura added 7 commits September 3, 2018 14:17

Add tests for metrics trigger

d14e530

Implement generic autoscaler

8424fc5

Add LegacyTrigger to autoscaler

3359270

Implement legacy type tests

9609715

Refactor roomStatus.Available() method to return Total() - (rooms on …

707d4f8

…terminating status)

Fix dev-room to exit() on sigterm and sigint

9e3f873

Implement autoscaler by cpu and mem

26e63e0

cscatolini reviewed Sep 6, 2018

View reviewed changes

henrod reviewed Sep 10, 2018

View reviewed changes

Luiz Felipe Takakura added 7 commits September 10, 2018 14:41

Implement basic cpu and mem autoscaler unit tests

c6f3ff4

Implement watcher tests for resource metrics

db046df

Correct ident

16b0c05

Update travis go version to 1.11

dfe228e

Remove focus on only one context

fca13cd

Implement tests for resources configure on scheduler

2d2787c

Update go version to 1.11

5289a40

lftakakura force-pushed the feature/auto-scaling-with-usage branch from 9ef73ac to 5289a40 Compare September 13, 2018 13:38

Luiz Felipe Takakura added 3 commits September 13, 2018 10:50

Remove test focus

c715a45

Update documentation

e786ea0

Change autoscaling flow on cooldown

da8e2af

lftakakura changed the title ~~WIP: Feature/auto scaling with usage~~ Feature/auto scaling with usage Sep 13, 2018

Luiz Felipe Takakura added 3 commits September 13, 2018 19:19

Fix validateMetricsTrigger method

68d7585

Implement cpu and mem increase routes on dev-room

478b959

Add logs to watcher

0a4d73b

henrod approved these changes Sep 13, 2018

View reviewed changes

lftakakura merged commit 5f1bd55 into master Sep 13, 2018

lftakakura deleted the feature/auto-scaling-with-usage branch September 13, 2018 22:57

cscatolini reviewed Sep 14, 2018

View reviewed changes

dev-room/api/increaseResourceUsage.py

@@ -0,0 +1,32 @@

import falcon

Copy link

Contributor

cscatolini Sep 14, 2018

Choose a reason for hiding this comment

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

@lftakakura this is the best file in the project 🤣

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/auto scaling with usage #47

Feature/auto scaling with usage #47

lftakakura commented Aug 28, 2018 •

edited

coveralls commented Aug 28, 2018 •

edited

henrod left a comment

henrod Aug 29, 2018

cscatolini Aug 29, 2018

henrod Aug 30, 2018

henrod Aug 29, 2018

henrod Aug 29, 2018

cscatolini Aug 29, 2018

cscatolini left a comment

cscatolini Aug 29, 2018

lftakakura Aug 29, 2018

cscatolini Aug 29, 2018

lftakakura Aug 29, 2018

cscatolini Aug 29, 2018

cscatolini Aug 29, 2018

lftakakura Aug 29, 2018

cscatolini Aug 29, 2018

cscatolini Aug 29, 2018

cscatolini left a comment

cscatolini Aug 30, 2018

lftakakura commented Aug 31, 2018 •

edited

henrod Aug 31, 2018

lftakakura Sep 3, 2018

henrod Aug 31, 2018

henrod Aug 31, 2018

henrod Aug 31, 2018

lftakakura Sep 4, 2018

cscatolini Sep 6, 2018

cscatolini Sep 6, 2018

cscatolini Sep 6, 2018

henrod Sep 10, 2018

henrod Sep 13, 2018

cscatolini Sep 14, 2018

Feature/auto scaling with usage #47

Feature/auto scaling with usage #47

Conversation

lftakakura commented Aug 28, 2018 • edited

coveralls commented Aug 28, 2018 • edited

henrod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cscatolini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cscatolini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftakakura commented Aug 31, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftakakura commented Aug 28, 2018 •

edited

coveralls commented Aug 28, 2018 •

edited

lftakakura commented Aug 31, 2018 •

edited