From 896f4b41f5a02aba5f4f42f24391cd937daf097e Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 12 Nov 2024 15:16:00 -0600 Subject: [PATCH 1/3] feat: Add jigasi region to debug output --- .../main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt index 1f24fd1917..d33d5912da 100644 --- a/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt +++ b/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt @@ -86,6 +86,7 @@ open class JigasiDetector( this["supports_transcription"] = instance.supportsTranscription() this["is_in_graceful_shutdown"] = instance.isInGracefulShutdown() this["participants"] = instance.getParticipantCount() + this["region"] = instance.getRegion() ?: "null" } debugState[instance.jid.resourceOrEmpty.toString()] = instanceJson } @@ -148,8 +149,10 @@ private fun BaseBrewery.BrewInstance.supportsTranscriptio private fun BaseBrewery.BrewInstance.supportsSip(): Boolean = java.lang.Boolean.parseBoolean(status.getValueAsString(ColibriStatsExtension.SUPPORTS_SIP)) private fun BaseBrewery.BrewInstance.isInRegion(vararg regions: String?): Boolean = - regions.contains(status.getValueAsString(ColibriStatsExtension.REGION)) + regions.contains(this.getRegion()) private fun BaseBrewery.BrewInstance.getParticipantCount(): Int = status.getValueAsInt(ColibriStatsExtension.PARTICIPANTS) ?: 0 +private fun BaseBrewery.BrewInstance.getRegion(): String? = + status.getValueAsString(ColibriStatsExtension.REGION) private fun List.BrewInstance>.leastLoaded() = minByOrNull { it.getParticipantCount() } From 0090c0c1f4612c103b6feacf672845a32c528f84 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 12 Nov 2024 16:13:21 -0600 Subject: [PATCH 2/3] feat: Support region groups for jigasi selection. --- .../org/jitsi/jicofo/bridge/BridgeConfig.kt | 14 ++++++++-- .../jicofo/bridge/BridgeSelectionStrategy.kt | 28 ++++--------------- .../RegionBasedBridgeSelectionStrategy.kt | 2 +- .../jitsi/jicofo/bridge/BridgeConfigTest.kt | 12 ++++++-- .../org/jitsi/jicofo/jigasi/JigasiDetector.kt | 9 ++++++ 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt index 6ee6222239..6bc1d970fb 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt @@ -148,17 +148,27 @@ class BridgeConfig private constructor() { "jicofo.bridge.xmpp-connection-name".from(JitsiConfig.newConfig) } - val regionGroups: Set> by config { + val regionGroups: Map> by config { "jicofo.bridge".from(JitsiConfig.newConfig).convertFrom { val regionGroupsConfigList = it["region-groups"] as? ConfigList ?: emptyList() - regionGroupsConfigList.map { regionsConfigList -> + val regionGroups = regionGroupsConfigList.map { regionsConfigList -> (regionsConfigList as? ConfigList ?: emptyList()).map { region -> region.unwrapped().toString() }.toSet() }.toSet() + mutableMapOf>().apply { + regionGroups.forEach { regionGroup -> + regionGroup.forEach { region -> + this[region] = regionGroup + } + } + } } } + fun getRegionGroup(region: String?): Set = + if (region == null) emptySet() else regionGroups[region] ?: setOf(region) + companion object { const val BASE = "jicofo.bridge" 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 6c2155d861..bbc0d75f2c 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 @@ -18,6 +18,7 @@ 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.jicofo.bridge.BridgeConfig.Companion.config as config /** * Represents an algorithm for bridge selection. @@ -88,8 +89,7 @@ abstract class BridgeSelectionStrategy { /** * Maximum participants per bridge in one conference, or `-1` for no maximum. */ - private val maxParticipantsPerBridge = - BridgeConfig.config.maxBridgeParticipants() + private val maxParticipantsPerBridge = config.maxBridgeParticipants() /** * Selects a bridge to be used for a new participant in a conference. @@ -176,7 +176,7 @@ abstract class BridgeSelectionStrategy { participantProperties: ParticipantProperties, desiredRegion: String? ): Bridge? { - val regionGroup = getRegionGroup(desiredRegion) + val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges .filterNot { isOverloaded(it, conferenceBridges) } .intersect(conferenceBridges.keys) @@ -234,7 +234,7 @@ abstract class BridgeSelectionStrategy { participantProperties: ParticipantProperties, desiredRegion: String? ): Bridge? { - val regionGroup = getRegionGroup(desiredRegion) + val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges .filterNot { isOverloaded(it, conferenceBridges) } .firstOrNull { regionGroup.contains(it.region) } @@ -279,7 +279,7 @@ abstract class BridgeSelectionStrategy { participantProperties: ParticipantProperties, desiredRegion: String? ): Bridge? { - val regionGroup = getRegionGroup(desiredRegion) + val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges .intersect(conferenceBridges.keys) .firstOrNull { regionGroup.contains(it.region) } @@ -320,7 +320,7 @@ abstract class BridgeSelectionStrategy { participantProperties: ParticipantProperties, desiredRegion: String? ): Bridge? { - val regionGroup = getRegionGroup(desiredRegion) + val regionGroup = config.getRegionGroup(desiredRegion) val result = bridges .firstOrNull { regionGroup.contains(it.region) } if (result != null) { @@ -407,22 +407,6 @@ abstract class BridgeSelectionStrategy { ) } - private val regionGroups: MutableMap> = HashMap() - - init { - BridgeConfig.config.regionGroups.forEach { regionGroup -> - regionGroup.forEach { region -> - regionGroups[region] = regionGroup - } - } - } - - protected fun getRegionGroup(region: String?): Set { - if (region == null) return setOf() - val regionGroup = regionGroups[region] - return regionGroup ?: setOf(region) - } - val stats: JSONObject get() { val json = JSONObject() 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 c90d798590..17bf4c9017 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 @@ -72,7 +72,7 @@ class RegionBasedBridgeSelectionStrategy : BridgeSelectionStrategy() { val participantRegion = participantProperties.region var region = participantRegion ?: localRegion if (localRegion != null) { - val regionGroup = getRegionGroup(region) + val regionGroup = BridgeConfig.config.getRegionGroup(region) if (conferenceBridges.isEmpty() && region != 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 diff --git a/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeConfigTest.kt b/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeConfigTest.kt index 900eaa872a..b32d8229b9 100644 --- a/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeConfigTest.kt +++ b/jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeConfigTest.kt @@ -82,7 +82,7 @@ class BridgeConfigTest : ShouldSpec() { } } context("Region groups") { - config.regionGroups shouldBe emptySet() + config.regionGroups shouldBe emptyMap() withNewConfig( """ @@ -92,7 +92,15 @@ class BridgeConfigTest : ShouldSpec() { ] """.trimIndent() ) { - config.regionGroups shouldBe setOf(setOf("us-east", "us-west"), setOf("eu-central", "eu-west")) + config.regionGroups shouldBe mapOf( + "us-east" to setOf("us-east", "us-west"), + "us-west" to setOf("us-east", "us-west"), + "eu-central" to setOf("eu-central", "eu-west"), + "eu-west" to setOf("eu-central", "eu-west") + ) + config.getRegionGroup(null) shouldBe emptySet() + config.getRegionGroup("abc") shouldBe setOf("abc") + config.getRegionGroup("us-east") shouldBe setOf("us-east", "us-west") } } } diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt index d33d5912da..8880fcadd4 100644 --- a/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt +++ b/jicofo/src/main/kotlin/org/jitsi/jicofo/jigasi/JigasiDetector.kt @@ -18,6 +18,7 @@ package org.jitsi.jicofo.jigasi import org.jitsi.jicofo.JicofoConfig +import org.jitsi.jicofo.bridge.BridgeConfig import org.jitsi.jicofo.metrics.JicofoMetricsContainer import org.jitsi.jicofo.xmpp.BaseBrewery import org.jitsi.jicofo.xmpp.XmppProvider @@ -119,6 +120,14 @@ open class JigasiDetector( availableInstances.filter { it.isInRegion(*preferredRegions.toTypedArray()) }.let { if (it.isNotEmpty()) return it.leastLoaded()?.jid } + // Try to match the preferred region groups. + val extendedPreferredRegions = preferredRegions.flatMap { region -> + BridgeConfig.config.getRegionGroup(region) + } + availableInstances.filter { it.isInRegion(*extendedPreferredRegions.toTypedArray()) }.let { + if (it.isNotEmpty()) return it.leastLoaded()?.jid + } + // Otherwise try to match the local region. availableInstances.filter { it.isInRegion(localRegion) }.let { if (it.isNotEmpty()) return it.leastLoaded()?.jid From cd92fa590b6f00aaea03792d520a2eea7f3bc5cc Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 12 Nov 2024 16:17:54 -0600 Subject: [PATCH 3/3] ref: Remove unnecessary functions. --- .../kotlin/org/jitsi/jicofo/bridge/Bridge.kt | 17 ++++++----------- .../org/jitsi/jicofo/bridge/BridgeConfig.kt | 12 +++++------- .../jicofo/bridge/BridgeSelectionStrategy.kt | 9 ++------- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/Bridge.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/Bridge.kt index 987c6dff53..c1f27ddc63 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/Bridge.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/Bridge.kt @@ -27,6 +27,7 @@ import org.jxmpp.jid.Jid import java.time.Clock import java.time.Duration import java.time.Instant +import org.jitsi.jicofo.bridge.BridgeConfig.Companion.config as config /** * Represents a jitsi-videobridge instance, reachable at a certain JID, which @@ -51,7 +52,7 @@ class Bridge @JvmOverloads internal constructor( * Keep track of the recently added endpoints. */ private val newEndpointsRate = RateTracker( - BridgeConfig.config.participantRampupInterval(), + config.participantRampupInterval, Duration.ofMillis(100), clock ) @@ -93,7 +94,7 @@ class Bridge @JvmOverloads internal constructor( // To filter out intermittent failures, do not return operational // until past the reset threshold since the last failure. if (failureInstant != null && - Duration.between(failureInstant, clock.instant()).compareTo(failureResetThreshold) < 0 + Duration.between(failureInstant, clock.instant()).compareTo(config.failureResetThreshold) < 0 ) { false } else { @@ -110,7 +111,7 @@ class Bridge @JvmOverloads internal constructor( /** * Start out with the configured value, update if the bridge reports a value. */ - private var averageParticipantStress = BridgeConfig.config.averageParticipantStress() + private var averageParticipantStress = config.averageParticipantStress /** * Stores a boolean that indicates whether the bridge is in graceful shutdown mode. @@ -216,7 +217,7 @@ class Bridge @JvmOverloads internal constructor( val healthy = stats.getValueAsString("healthy") if (healthy != null) { isHealthy = java.lang.Boolean.parseBoolean(healthy) - } else if (BridgeConfig.config.usePresenceForHealth) { + } else if (config.usePresenceForHealth) { logger.warn( "Presence-based health checks are enabled, but presence did not include health status. Health " + "checks for this bridge are effectively disabled." @@ -286,7 +287,7 @@ class Bridge @JvmOverloads internal constructor( * @return true if the stress of the bridge is greater-than-or-equal to the threshold. */ val isOverloaded: Boolean - get() = stress >= BridgeConfig.config.stressThreshold() + get() = stress >= config.stressThreshold val debugState: OrderedJsonObject get() { @@ -306,12 +307,6 @@ class Bridge @JvmOverloads internal constructor( } companion object { - /** - * How long the "failed" state should be sticky for. Once a [Bridge] goes in a non-operational state (via - * [.setIsOperational]) it will be considered non-operational for at least this amount of time. - * See the tests for example behavior. - */ - private val failureResetThreshold = BridgeConfig.config.failureResetThreshold() /** * Returns a negative number if b1 is more able to serve conferences than b2. The computation is based on the diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt index 6bc1d970fb..9d0b9f5614 100644 --- a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt +++ b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeConfig.kt @@ -36,40 +36,38 @@ class BridgeConfig private constructor() { "org.jitsi.jicofo.BridgeSelector.MAX_PARTICIPANTS_PER_BRIDGE".from(JitsiConfig.legacyConfig) "$BASE.max-bridge-participants".from(JitsiConfig.newConfig) } - fun maxBridgeParticipants() = maxBridgeParticipants val maxBridgePacketRatePps: Int by config { "org.jitsi.jicofo.BridgeSelector.MAX_BRIDGE_PACKET_RATE".from(JitsiConfig.legacyConfig) "$BASE.max-bridge-packet-rate".from(JitsiConfig.newConfig) } - fun maxBridgePacketRatePps() = maxBridgePacketRatePps val averageParticipantPacketRatePps: Int by config { "org.jitsi.jicofo.BridgeSelector.AVG_PARTICIPANT_PACKET_RATE".from(JitsiConfig.legacyConfig) "$BASE.average-participant-packet-rate-pps" .from(JitsiConfig.newConfig).softDeprecated("use $BASE.average-participant-stress") } - fun averageParticipantPacketRatePps() = averageParticipantPacketRatePps val averageParticipantStress: Double by config { "$BASE.average-participant-stress".from(JitsiConfig.newConfig) } - fun averageParticipantStress() = averageParticipantStress val stressThreshold: Double by config { "$BASE.stress-threshold".from(JitsiConfig.newConfig) } - fun stressThreshold() = stressThreshold + /** + * How long the "failed" state should be sticky for. Once a [Bridge] goes in a non-operational state (via + * [.setIsOperational]) it will be considered non-operational for at least this amount of time. + * See the tests for example behavior. + */ val failureResetThreshold: Duration by config { "org.jitsi.focus.BRIDGE_FAILURE_RESET_THRESHOLD".from(JitsiConfig.legacyConfig) .convertFrom { Duration.ofMillis(it) } "$BASE.failure-reset-threshold".from(JitsiConfig.newConfig) } - fun failureResetThreshold() = failureResetThreshold val participantRampupInterval: Duration by config { "$BASE.participant-rampup-interval".from(JitsiConfig.newConfig) } - fun participantRampupInterval() = participantRampupInterval val selectionStrategy: BridgeSelectionStrategy by config { "org.jitsi.jicofo.BridgeSelector.BRIDGE_SELECTION_STRATEGY".from(JitsiConfig.legacyConfig) 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 bbc0d75f2c..642620a60a 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 @@ -86,11 +86,6 @@ abstract class BridgeSelectionStrategy { */ private var totalLeastLoaded = 0 - /** - * Maximum participants per bridge in one conference, or `-1` for no maximum. - */ - private val maxParticipantsPerBridge = config.maxBridgeParticipants() - /** * Selects a bridge to be used for a new participant in a conference. * @@ -401,9 +396,9 @@ abstract class BridgeSelectionStrategy { */ private fun isOverloaded(bridge: Bridge, conferenceBridges: Map): Boolean { return bridge.isOverloaded || ( - maxParticipantsPerBridge > 0 && + config.maxBridgeParticipants > 0 && conferenceBridges.containsKey(bridge) && - conferenceBridges[bridge]!!.participantCount >= maxParticipantsPerBridge + conferenceBridges[bridge]!!.participantCount >= config.maxBridgeParticipants ) }