From f6b3764446c2a841667e92cb59b8a926c5fc09d7 Mon Sep 17 00:00:00 2001
From: calvin681 <cal681@gmail.com>
Date: Tue, 5 Jan 2021 11:48:40 -0800
Subject: [PATCH] Rope uses percentage for Clutch RPS (#86)

* Rope uses absolute values instead of percentage

Co-authored-by: Calvin Cheung <ccheung@netflix.com>
---
 .../worker/jobmaster/JobAutoScaler.java       | 37 ++++++++++---------
 .../rps/RpsClutchConfigurationSelector.java   |  3 +-
 .../RpsClutchConfigurationSelectorTest.java   |  4 +-
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/JobAutoScaler.java b/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/JobAutoScaler.java
index 073cc053c..6eb201370 100644
--- a/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/JobAutoScaler.java
+++ b/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/JobAutoScaler.java
@@ -149,14 +149,32 @@ void start() {
                             clutchCustomConfiguration.isPresent())) {
 
                         ClutchConfiguration config = null;
-                        int minSize = stageSchedulingInfo.getScalingPolicy().getMin();
-                        int maxSize = stageSchedulingInfo.getScalingPolicy().getMax();
+                        int minSize = 0;
+                        int maxSize = 0;
                         boolean useJsonConfigBased = false;
                         boolean useClutch = false;
                         boolean useClutchRps = false;
                         boolean useClutchExperimental = false;
 
                         // Determine which type of scaler to use.
+                        if (stageSchedulingInfo.getScalingPolicy() != null) {
+                            minSize = stageSchedulingInfo.getScalingPolicy().getMin();
+                            maxSize = stageSchedulingInfo.getScalingPolicy().getMax();
+                            if (stageSchedulingInfo.getScalingPolicy().getStrategies() != null) {
+                                Set<StageScalingPolicy.ScalingReason> reasons = stageSchedulingInfo.getScalingPolicy().getStrategies()
+                                        .values()
+                                        .stream()
+                                        .map(StageScalingPolicy.Strategy::getReason)
+                                        .collect(Collectors.toSet());
+                                if (reasons.contains(StageScalingPolicy.ScalingReason.Clutch)) {
+                                    useClutch = true;
+                                } else if (reasons.contains(StageScalingPolicy.ScalingReason.ClutchExperimental)) {
+                                    useClutchExperimental = true;
+                                } else if (reasons.contains(StageScalingPolicy.ScalingReason.ClutchRps)) {
+                                    useClutchRps = true;
+                                }
+                            }
+                        }
                         if (clutchCustomConfiguration.isPresent()) {
                             try {
                                 config = getClutchConfiguration(clutchCustomConfiguration.get()).get(stage);
@@ -179,21 +197,6 @@ void start() {
                                 }
                             }
                         }
-                        if (stageSchedulingInfo.getScalingPolicy() != null &&
-                                stageSchedulingInfo.getScalingPolicy().getStrategies() != null) {
-                            Set<StageScalingPolicy.ScalingReason> reasons = stageSchedulingInfo.getScalingPolicy().getStrategies()
-                                    .values()
-                                    .stream()
-                                    .map(StageScalingPolicy.Strategy::getReason)
-                                    .collect(Collectors.toSet());
-                            if (reasons.contains(StageScalingPolicy.ScalingReason.Clutch)) {
-                                useClutch = true;
-                            } else if (reasons.contains(StageScalingPolicy.ScalingReason.ClutchExperimental)) {
-                                useClutchExperimental = true;
-                            } else if (reasons.contains(StageScalingPolicy.ScalingReason.ClutchRps)) {
-                                useClutchRps = true;
-                            }
-                        }
 
                         int initialSize = stageSchedulingInfo.getNumberOfInstances();
                         StageScaler scaler = new StageScaler(stage, stageSchedulingInfo);
diff --git a/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelector.java b/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelector.java
index 1daa95230..a22e7eb8a 100644
--- a/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelector.java
+++ b/mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelector.java
@@ -42,6 +42,7 @@ public RpsClutchConfigurationSelector(Integer stageNumber, StageSchedulingInfo s
     @Override
     public ClutchConfiguration apply(Map<Clutch.Metric, UpdateDoublesSketch> sketches) {
         double setPoint = getSetpoint(sketches);
+        Tuple2<Double, Double> rope = getRope().map(x -> x * setPoint, y -> y * setPoint);
 
         // Gain - number of ticks within the cooldown period. This is the minimum number of times PID output will accumulate
         // before an action is taken.
@@ -60,7 +61,7 @@ public ClutchConfiguration apply(Map<Clutch.Metric, UpdateDoublesSketch> sketche
                 .integralDecay(INTEGRAL_DECAY)
                 .minSize(getMinSize())
                 .maxSize(getMaxSize())
-                .rope(getRope())
+                .rope(rope)
                 .cooldownInterval(getCooldownSecs())
                 .cooldownUnits(TimeUnit.SECONDS)
                 .build();
diff --git a/mantis-server/mantis-server-worker/src/test/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelectorTest.java b/mantis-server/mantis-server-worker/src/test/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelectorTest.java
index 99fae422e..2af74238b 100644
--- a/mantis-server/mantis-server-worker/src/test/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelectorTest.java
+++ b/mantis-server/mantis-server-worker/src/test/java/io/mantisrx/server/worker/jobmaster/clutch/rps/RpsClutchConfigurationSelectorTest.java
@@ -55,7 +55,7 @@ public void testApply() {
         assertEquals(100.0, config.getSetPoint(), 1e-10);
         assertEquals(1, config.getMinSize());
         assertEquals(10, config.getMaxSize());
-        assertEquals(Tuple.of(0.2, 0.1), config.getRope());
+        assertEquals(Tuple.of(20.0, 10.0), config.getRope());
         assertEquals(300L, config.getCooldownInterval());
     }
 
@@ -76,7 +76,7 @@ public void testScalingPolicyFallback() {
         assertEquals(100.0, config.getSetPoint(), 1e-10);
         assertEquals(2, config.getMinSize());
         assertEquals(9, config.getMaxSize());
-        assertEquals(Tuple.of(0.3, 0.0), config.getRope());
+        assertEquals(Tuple.of(30.0, 0.0), config.getRope());
         assertEquals(400L, config.getCooldownInterval());
 
     }