batch-scheduler: fix the number of cross-vm links computation (#367)

* batch-scheduler: fix the number of cross-vm links computation * chore: bump patch code version
faasm · Jan 25, 2024 · 62c022d · 62c022d
1 parent d7d8e97
commit 62c022d
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 12 deletions.
diff --git a/.env b/.env
@@ -1,4 +1,4 @@
-FAABRIC_VERSION=0.13.0
-FAABRIC_CLI_IMAGE=faasm.azurecr.io/faabric:0.13.0
+FAABRIC_VERSION=0.13.1
+FAABRIC_CLI_IMAGE=faasm.azurecr.io/faabric:0.13.1
 COMPOSE_PROJECT_NAME=faabric-dev
 CONAN_CACHE_MOUNT_SOURCE=./conan-cache/
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -20,7 +20,7 @@ jobs:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}
@@ -33,7 +33,7 @@ jobs:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}
@@ -47,7 +47,7 @@ jobs:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}
@@ -70,7 +70,7 @@ jobs:
       REDIS_QUEUE_HOST: redis
       REDIS_STATE_HOST: redis
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}
@@ -110,7 +110,7 @@ jobs:
       REDIS_QUEUE_HOST: redis
       REDIS_STATE_HOST: redis
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}
@@ -164,7 +164,7 @@ jobs:
       REDIS_QUEUE_HOST: redis
       REDIS_STATE_HOST: redis
     container:
-      image: faasm.azurecr.io/faabric:0.13.0
+      image: faasm.azurecr.io/faabric:0.13.1
       credentials:
         username: ${{ secrets.ACR_SERVICE_PRINCIPAL_ID }}
         password: ${{ secrets.ACR_SERVICE_PRINCIPAL_PASSWORD }}

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-0.13.0
+0.13.1
diff --git a/src/batch-scheduler/BinPackScheduler.cpp b/src/batch-scheduler/BinPackScheduler.cpp
@@ -102,6 +102,11 @@ bool BinPackScheduler::isFirstDecisionBetter(
   std::shared_ptr<SchedulingDecision> decisionA,
   std::shared_ptr<SchedulingDecision> decisionB)
 {
+    // The locality score is currently the number of cross-VM links. You may
+    // calculate this number as follows:
+    // - If the decision is single host, the number of cross-VM links is zero
+    // - Otherwise, in a fully-connected graph, the number of cross-VM links
+    //   is the sum of edges that cross a VM boundary
     auto getLocalityScore =
       [](std::shared_ptr<SchedulingDecision> decision) -> std::pair<int, int> {
         // First, calculate the host-message histogram (or frequency count)
@@ -115,12 +120,22 @@ bool BinPackScheduler::isFirstDecisionBetter(
             return std::make_pair(1, 0);
         }
 
-        // Else, do the product of all entries
-        int score = 1;
+        // Else, sum all the egressing edges for each element and divide by two
+        int score = 0;
         for (auto [host, freq] : hostFreqCount) {
-            score = score * freq;
+
+            int thisHostScore = 0;
+            for (auto [innerHost, innerFreq] : hostFreqCount) {
+                if (innerHost != host) {
+                    thisHostScore += innerFreq;
+                }
+            }
+
+            score += thisHostScore * freq;
         }
 
+        score = int(score / 2);
+
         return std::make_pair(hostFreqCount.size(), score);
     };
 

diff --git a/tests/test/batch-scheduler/test_binpack_scheduler.cpp b/tests/test/batch-scheduler/test_binpack_scheduler.cpp
@@ -452,6 +452,26 @@ TEST_CASE_METHOD(BinPackSchedulerTestFixture,
           ber, { "foo", "foo", "foo", "bar", "bar", "foo" });
     }
 
+    // Check we correctly minimise cross-VM links in >2 VM scenarios
+    SECTION("It also minimises cross-VM links with more than 2 VMs")
+    {
+        config.hostMap = buildHostMap(
+          {
+            "foo",
+            "bar",
+            "baz",
+            "bat",
+          },
+          { 2, 2, 1, 1 },
+          { 1, 1, 1, 1 });
+        ber = faabric::util::batchExecFactory("bat", "man", 4);
+        ber->set_type(BatchExecuteRequest_BatchExecuteType_MIGRATION);
+        config.inFlightReqs =
+          buildInFlightReqs(ber, 4, { "foo", "bar", "baz", "bat" });
+        config.expectedDecision =
+          buildExpectedDecision(ber, { "foo", "bar", "bar", "foo" });
+    }
+
     SECTION("BinPack will minimise the number of messages to migrate")
     {
         config.hostMap =