Prefer local region for initial selection (#936)

* feat: Default to localRegion when participantRegion is null. * test: Add a test for initial selection in region group. * feat: Prefer local region for initial selection. * doc: Cleanup docs.
jitsi · Jun 8, 2022 · beca273 · beca273
1 parent a658cd1
commit beca273
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 14 deletions.
diff --git a/src/main/java/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.java b/src/main/java/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.java
@@ -16,6 +16,7 @@
 package org.jitsi.jicofo.bridge;
 
 import org.jetbrains.annotations.*;
+import org.jitsi.jicofo.*;
 import org.jitsi.utils.logging2.*;
 
 import java.util.*;
@@ -93,18 +94,39 @@ public Bridge doSelect(
             return null;
         }
 
-        return notLoadedAlreadyInConferenceInRegion(bridges, conferenceBridges, participantRegion).orElseGet(
+        String localRegion = JicofoConfig.config.localRegion();
+        String r = participantRegion == null ? localRegion : participantRegion;
+        if (localRegion != null)
+        {
+            if (conferenceBridges.isEmpty() && !Objects.equals(r, JicofoConfig.config.localRegion()))
+            {
+                // Selecting an initial bridge for a participant not in the local region. This is most likely because
+                // exactly one of the first two participants in the conference is not in the local region, and we're
+                // selecting for it first. I.e. there is another participant in the local region which will be
+                // subsequently invited.
+                Set<String> regionGroup = regionGroups.get(r);
+                if (regionGroup != null && regionGroup.contains(localRegion))
+                {
+                    // With the above assumption, there are two participants in the local region group. Therefore,
+                    // they will use the same bridge. Prefer to use a bridge in the local region.
+                    r = localRegion;
+                }
+            }
+        }
+
+        final String region = r;
+        return notLoadedAlreadyInConferenceInRegion(bridges, conferenceBridges, region).orElseGet(
                 () -> notLoadedAlreadyInConferenceInRegionGroup(
-                        bridges, conferenceBridges, getRegionGroup(participantRegion)).orElseGet(
-                () -> notLoadedInRegion(bridges, conferenceBridges, participantRegion).orElseGet(
-                () -> notLoadedInRegionGroup(bridges, conferenceBridges, getRegionGroup(participantRegion)).orElseGet(
-                () -> leastLoadedAlreadyInConferenceInRegion(bridges, conferenceBridges, participantRegion).orElseGet(
+                        bridges, conferenceBridges, getRegionGroup(region)).orElseGet(
+                () -> notLoadedInRegion(bridges, conferenceBridges, region).orElseGet(
+                () -> notLoadedInRegionGroup(bridges, conferenceBridges, getRegionGroup(region)).orElseGet(
+                () -> leastLoadedAlreadyInConferenceInRegion(bridges, conferenceBridges, region).orElseGet(
                 () -> leastLoadedAlreadyInConferenceInRegionGroup(
-                        bridges, conferenceBridges, getRegionGroup(participantRegion)).orElseGet(
-                () -> leastLoadedInRegion(bridges, conferenceBridges, participantRegion).orElseGet(
-                () -> leastLoadedInRegionGroup(bridges, conferenceBridges, getRegionGroup(participantRegion)).orElseGet(
-                () -> nonLoadedAlreadyInConference(bridges, conferenceBridges, participantRegion).orElseGet(
-                () -> leastLoaded(bridges, conferenceBridges, participantRegion).orElse(null))))))))));
+                        bridges, conferenceBridges, getRegionGroup(region)).orElseGet(
+                () -> leastLoadedInRegion(bridges, conferenceBridges, region).orElseGet(
+                () -> leastLoadedInRegionGroup(bridges, conferenceBridges, getRegionGroup(region)).orElseGet(
+                () -> nonLoadedAlreadyInConference(bridges, conferenceBridges, region).orElseGet(
+                () -> leastLoaded(bridges, conferenceBridges, region).orElse(null))))))))));
     }
 
     @Override

diff --git a/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt b/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt
@@ -105,11 +105,9 @@ class BridgeSelector @JvmOverloads constructor(
     }
 
     /**
-     * Removes Jitsi Videobridge XMPP address from the list videobridge
-     * instances available in the system .
+     * Removes a [Bridge] with a specific JID from the list of videobridge instances.
      *
-     * @param bridgeJid the JID of videobridge to be removed from this selector's
-     * set of videobridges.
+     * @param bridgeJid the JID of bridge to remove.
      */
     @Synchronized
     fun removeJvbAddress(bridgeJid: Jid) {

diff --git a/src/test/kotlin/org/jitsi/jicofo/bridge/RegionBasedSelectionTest.kt b/src/test/kotlin/org/jitsi/jicofo/bridge/RegionBasedSelectionTest.kt
@@ -152,6 +152,9 @@ class RegionBasedSelectionTest : ShouldSpec() {
                             bridges[EuCentral][Low] to maxBp
                         )
                     ) shouldBe bridges[UsEast][Low]
+                    context("Initial selection in the local region group, but not in the local region") {
+                        select(participantRegion = UsWest, conferenceBridges = emptyMap()) shouldBe bridges[UsEast][Low]
+                    }
                 }
             }
         }
@@ -161,6 +164,7 @@ class RegionBasedSelectionTest : ShouldSpec() {
 private const val maxBp = 10
 const val maxBpConfig = "jicofo.bridge.max-bridge-participants=$maxBp"
 val regionGroupsConfig = """
+    jicofo.local-region = ${UsEast.region}
     jicofo.bridge.selection-strategy=RegionBasedBridgeSelectionStrategy
     jicofo.bridge.region-groups = [
        [ "${UsEast.region}", "${UsWest.region}" ],