diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategy.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategy.kt index 642620a60a..b0f140d789 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategy.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategy.kt @@ -16,75 +16,14 @@ package org.jitsi.jicofo.bridge import org.jitsi.utils.logging2.Logger -import org.jitsi.utils.logging2.LoggerImpl -import org.json.simple.JSONObject +import org.jitsi.utils.logging2.createLogger import org.jitsi.jicofo.bridge.BridgeConfig.Companion.config as config /** * Represents an algorithm for bridge selection. */ abstract class BridgeSelectionStrategy { - /** - * Total number of times selection succeeded because there was a bridge - * already in the conference, in the desired region that was not - * overloaded. - */ - private var totalNotLoadedAlreadyInConferenceInRegion = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * already in the conference, in the desired region group that was not - * overloaded. - */ - private var totalNotLoadedAlreadyInConferenceInRegionGroup = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * in the desired region that was not overloaded. - */ - private var totalNotLoadedInRegion = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * in the desired region group that was not overloaded. - */ - private var totalNotLoadedInRegionGroup = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * already in the conference, in the desired region. - */ - private var totalLeastLoadedAlreadyInConferenceInRegion = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * already in the conference, in the desired region group. - */ - private var totalLeastLoadedAlreadyInConferenceInRegionGroup = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * in the desired region. - */ - private var totalLeastLoadedInRegion = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * in the desired region group. - */ - private var totalLeastLoadedInRegionGroup = 0 - - /** - * Total number of times selection succeeded because there was a bridge - * already in the conference. - */ - private var totalLeastLoadedAlreadyInConference = 0 - - /** - * Total number of times selection succeeded because there was any bridge - * available. - */ - private var totalLeastLoaded = 0 + private val logger: Logger = createLogger() /** * Selects a bridge to be used for a new participant in a conference. @@ -155,11 +94,10 @@ abstract class BridgeSelectionStrategy { desiredRegion: String? ): Bridge? { val result = bridges - .filterNot { isOverloaded(it, conferenceBridges) } + .filterNot { it.isOverloaded(conferenceBridges) } .intersect(conferenceBridges.keys) .firstOrNull { desiredRegion != null && it.region.equals(desiredRegion) } if (result != null) { - totalNotLoadedAlreadyInConferenceInRegion++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result @@ -173,11 +111,10 @@ abstract class BridgeSelectionStrategy { ): Bridge? { val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges - .filterNot { isOverloaded(it, conferenceBridges) } + .filterNot { it.isOverloaded(conferenceBridges) } .intersect(conferenceBridges.keys) .firstOrNull { regionGroup.contains(it.region) } if (result != null) { - totalNotLoadedAlreadyInConferenceInRegionGroup++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result @@ -214,10 +151,9 @@ abstract class BridgeSelectionStrategy { desiredRegion: String? ): Bridge? { val result = bridges - .filterNot { isOverloaded(it, conferenceBridges) } + .filterNot { it.isOverloaded(conferenceBridges) } .firstOrNull { desiredRegion != null && it.region.equals(desiredRegion) } if (result != null) { - totalNotLoadedInRegion++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result @@ -231,15 +167,26 @@ abstract class BridgeSelectionStrategy { ): Bridge? { val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges - .filterNot { isOverloaded(it, conferenceBridges) } + .filterNot { it.isOverloaded(conferenceBridges) } .firstOrNull { regionGroup.contains(it.region) } if (result != null) { - totalNotLoadedInRegionGroup++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result } + fun notLoaded( + bridges: List, + conferenceBridges: Map, + participantProperties: ParticipantProperties + ): Bridge? { + val result = bridges.firstOrNull { !it.isOverloaded } + if (result != null) { + logSelection(result, conferenceBridges, participantProperties) + } + return result + } + /** * Finds the least loaded conference bridge in the desired region that * is already handling the conference. @@ -262,25 +209,21 @@ abstract class BridgeSelectionStrategy { .intersect(conferenceBridges.keys) .firstOrNull { desiredRegion != null && it.region.equals(desiredRegion) } if (result != null) { - totalLeastLoadedAlreadyInConferenceInRegion++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result } - fun leastLoadedAlreadyInConferenceInRegionGroup( + fun leastLoadedNotMaxedAlreadyInConference( bridges: List, conferenceBridges: Map, - participantProperties: ParticipantProperties, - desiredRegion: String? + participantProperties: ParticipantProperties ): Bridge? { - val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges .intersect(conferenceBridges.keys) - .firstOrNull { regionGroup.contains(it.region) } + .firstOrNull { !it.hasMaxParticipantsInConference(conferenceBridges) } if (result != null) { - totalLeastLoadedAlreadyInConferenceInRegionGroup++ - logSelection(result, conferenceBridges, participantProperties, desiredRegion) + logSelection(result, conferenceBridges, participantProperties) } return result } @@ -303,23 +246,6 @@ abstract class BridgeSelectionStrategy { val result = bridges .firstOrNull { desiredRegion != null && it.region.equals(desiredRegion) } if (result != null) { - totalLeastLoadedInRegion++ - logSelection(result, conferenceBridges, participantProperties, desiredRegion) - } - return result - } - - fun leastLoadedInRegionGroup( - bridges: List, - conferenceBridges: Map, - participantProperties: ParticipantProperties, - desiredRegion: String? - ): Bridge? { - val regionGroup = config.getRegionGroup(desiredRegion) - val result = bridges - .firstOrNull { regionGroup.contains(it.region) } - if (result != null) { - totalLeastLoadedInRegionGroup++ logSelection(result, conferenceBridges, participantProperties, desiredRegion) } return result @@ -334,17 +260,16 @@ abstract class BridgeSelectionStrategy { * * @return the least loaded bridge that is already in the conference, if it exists, or null */ - fun nonLoadedAlreadyInConference( + fun notLoadedAlreadyInConference( bridges: List, conferenceBridges: Map, participantProperties: ParticipantProperties ): Bridge? { val result = bridges - .filterNot { isOverloaded(it, conferenceBridges) } + .filterNot { it.isOverloaded(conferenceBridges) } .intersect(conferenceBridges.keys) .firstOrNull() if (result != null) { - totalLeastLoadedAlreadyInConference++ logSelection(result, conferenceBridges, participantProperties) } return result @@ -364,7 +289,6 @@ abstract class BridgeSelectionStrategy { ): Bridge? { val result = bridges.firstOrNull() if (result != null) { - totalLeastLoaded++ logSelection(result, conferenceBridges, participantProperties) } return result @@ -394,36 +318,15 @@ abstract class BridgeSelectionStrategy { * @param conferenceBridges the bridges in the conference * @return `true` if the bridge should be considered overloaded. */ - private fun isOverloaded(bridge: Bridge, conferenceBridges: Map): Boolean { - return bridge.isOverloaded || ( - config.maxBridgeParticipants > 0 && - conferenceBridges.containsKey(bridge) && - conferenceBridges[bridge]!!.participantCount >= config.maxBridgeParticipants - ) + private fun Bridge.isOverloaded(conferenceBridges: Map): Boolean { + return isOverloaded || hasMaxParticipantsInConference(conferenceBridges) } - val stats: JSONObject - get() { - val json = JSONObject() - json["total_not_loaded_in_region_in_conference"] = totalNotLoadedAlreadyInConferenceInRegion - json["total_not_loaded_in_region_group_in_conference"] = totalNotLoadedAlreadyInConferenceInRegionGroup - json["total_not_loaded_in_region"] = totalNotLoadedInRegion - json["total_not_loaded_in_region_group"] = totalNotLoadedInRegionGroup - json["total_least_loaded_in_region_in_conference"] = totalLeastLoadedAlreadyInConferenceInRegion - json["total_least_loaded_in_region_group_in_conference"] = totalLeastLoadedAlreadyInConferenceInRegionGroup - json["total_least_loaded_in_region"] = totalLeastLoadedInRegion - json["total_least_loaded_in_region_group"] = totalLeastLoadedInRegionGroup - json["total_least_loaded_in_conference"] = totalLeastLoadedAlreadyInConference - json["total_least_loaded"] = totalLeastLoaded - return json - } - - companion object { - /** - * The logger. - */ - private val logger: Logger = LoggerImpl( - BridgeSelectionStrategy::class.java.name - ) + private fun Bridge.hasMaxParticipantsInConference( + conferenceBridges: Map + ): Boolean { + return config.maxBridgeParticipants > 0 && + conferenceBridges.containsKey(this) && + conferenceBridges[this]!!.participantCount >= config.maxBridgeParticipants } } diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt index 638d793980..c092575c91 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelector.kt @@ -222,7 +222,7 @@ class BridgeSelector @JvmOverloads constructor( val stats: JSONObject @Synchronized - get() = bridgeSelectionStrategy.stats.apply { + get() = JSONObject().apply { // We want to avoid exposing unnecessary hierarchy levels in the stats, // so we'll merge stats from different "child" objects here. this["bridge_count"] = bridgeCount.get() diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.kt index 17bf4c9017..93c9532096 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.kt @@ -94,11 +94,9 @@ class RegionBasedBridgeSelectionStrategy : BridgeSelectionStrategy() { ?: notLoadedAlreadyInConferenceInRegionGroup(bridges, conferenceBridges, participantProperties, region) ?: notLoadedInRegion(bridges, conferenceBridges, participantProperties, region) ?: notLoadedInRegionGroup(bridges, conferenceBridges, participantProperties, region) - ?: leastLoadedAlreadyInConferenceInRegion(bridges, conferenceBridges, participantProperties, region) - ?: leastLoadedAlreadyInConferenceInRegionGroup(bridges, conferenceBridges, participantProperties, region) - ?: leastLoadedInRegion(bridges, conferenceBridges, participantProperties, region) - ?: leastLoadedInRegionGroup(bridges, conferenceBridges, participantProperties, region) - ?: nonLoadedAlreadyInConference(bridges, conferenceBridges, participantProperties) + ?: notLoadedAlreadyInConference(bridges, conferenceBridges, participantProperties) + ?: notLoaded(bridges, conferenceBridges, participantProperties) + ?: leastLoadedNotMaxedAlreadyInConference(bridges, conferenceBridges, participantProperties) ?: leastLoaded(bridges, conferenceBridges, participantProperties) } diff --git a/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategyTest.kt b/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategyTest.kt index bb5f9e3fd3..e6b591be23 100644 --- a/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategyTest.kt +++ b/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategyTest.kt @@ -89,8 +89,9 @@ class BridgeSelectionStrategyTest : ShouldSpec() { val propsNull = ParticipantProperties(null) val conferenceBridges = mutableMapOf() - // Initial selection should select a bridge in the participant's region. - strategy.select(allBridges, conferenceBridges, highStressProps, true) shouldBe highStressBridge + // Initial selection should select a non-overloaded bridge in the participant's region if possible. If not, + // it should select the lowest loaded bridge. + strategy.select(allBridges, conferenceBridges, highStressProps, true) shouldBe lowStressBridge strategy.select(allBridges, conferenceBridges, mediumStressProps, true) shouldBe mediumStressBridge strategy.select(allBridges, conferenceBridges, propsInvalid, true) shouldBe lowStressBridge @@ -135,8 +136,9 @@ class BridgeSelectionStrategyTest : ShouldSpec() { val conferenceBridges = mutableMapOf() - // Initial selection should select a bridge in the participant's region. - strategy.select(allBridges, conferenceBridges, highStressProps, true) shouldBe highStressBridge + // Initial selection should select a non-overloaded bridge in the participant's region if possible. If not, + // it should select the lowest loaded bridge. + strategy.select(allBridges, conferenceBridges, highStressProps, true) shouldBe mediumStressBridge1 strategy.select(allBridges, conferenceBridges, mediumStressProps2, true) shouldBe mediumStressBridge2 strategy.select(allBridges, conferenceBridges, propsInvalid, true) shouldBe mediumStressBridge1 strategy.select(allBridges, conferenceBridges, propsNull, true) shouldBe mediumStressBridge1