Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prioritize load higher than region. #1206

Merged
merged 2 commits into from
Jan 24, 2025
Merged

fix: Prioritize load higher than region. #1206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5608837
fix: Prioritize load higher than region.
bgrozev Jan 24, 2025
2090dba
ref: Remove detailed selection stats (not useful).
bgrozev Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 34 additions & 49 deletions jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategy.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,12 @@ abstract class BridgeSelectionStrategy {
*/
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.
Expand Down Expand Up @@ -155,7 +143,7 @@ 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) {
Expand All @@ -173,7 +161,7 @@ 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) {
Expand Down Expand Up @@ -214,7 +202,7 @@ 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++
Expand All @@ -231,7 +219,7 @@ 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++
Expand All @@ -240,6 +228,19 @@ abstract class BridgeSelectionStrategy {
return result
}

fun notLoaded(
bridges: List<Bridge>,
conferenceBridges: Map<Bridge, ConferenceBridgeProperties>,
participantProperties: ParticipantProperties
): Bridge? {
val result = bridges.firstOrNull { !it.isOverloaded }
if (result != null) {
totalLeastLoadedAlreadyInConference++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stat also wants a new name

logSelection(result, conferenceBridges, participantProperties)
}
return result
}

/**
* Finds the least loaded conference bridge in the desired region that
* is already handling the conference.
Expand Down Expand Up @@ -268,19 +269,17 @@ abstract class BridgeSelectionStrategy {
return result
}

fun leastLoadedAlreadyInConferenceInRegionGroup(
fun leastLoadedNotMaxedAlreadyInConference(
bridges: List<Bridge>,
conferenceBridges: Map<Bridge, ConferenceBridgeProperties>,
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)
totalLeastLoadedAlreadyInConferenceInRegion++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stat probably wants a new name?

logSelection(result, conferenceBridges, participantProperties)
}
return result
}
Expand Down Expand Up @@ -309,22 +308,6 @@ abstract class BridgeSelectionStrategy {
return result
}

fun leastLoadedInRegionGroup(
bridges: List<Bridge>,
conferenceBridges: Map<Bridge, ConferenceBridgeProperties>,
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
}

/**
* Finds the least loaded non overloaded bridge that is already handling the conference.
*
Expand All @@ -334,13 +317,13 @@ 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<Bridge>,
conferenceBridges: Map<Bridge, ConferenceBridgeProperties>,
participantProperties: ParticipantProperties
): Bridge? {
val result = bridges
.filterNot { isOverloaded(it, conferenceBridges) }
.filterNot { it.isOverloaded(conferenceBridges) }
.intersect(conferenceBridges.keys)
.firstOrNull()
if (result != null) {
Expand Down Expand Up @@ -394,12 +377,16 @@ 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<Bridge, ConferenceBridgeProperties>): Boolean {
return bridge.isOverloaded || (
config.maxBridgeParticipants > 0 &&
conferenceBridges.containsKey(bridge) &&
conferenceBridges[bridge]!!.participantCount >= config.maxBridgeParticipants
)
private fun Bridge.isOverloaded(conferenceBridges: Map<Bridge, ConferenceBridgeProperties>): Boolean {
return isOverloaded || hasMaxParticipantsInConference(conferenceBridges)
}

private fun Bridge.hasMaxParticipantsInConference(
conferenceBridges: Map<Bridge, ConferenceBridgeProperties>
): Boolean {
return config.maxBridgeParticipants > 0 &&
conferenceBridges.containsKey(this) &&
conferenceBridges[this]!!.participantCount >= config.maxBridgeParticipants
}

val stats: JSONObject
Expand All @@ -410,9 +397,7 @@ abstract class BridgeSelectionStrategy {
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
Expand Down
8 changes: 3 additions & 5 deletions ...fo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/RegionBasedBridgeSelectionStrategy.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
10 changes: 6 additions & 4 deletions jicofo-selector/src/test/kotlin/org/jitsi/jicofo/bridge/BridgeSelectionStrategyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ class BridgeSelectionStrategyTest : ShouldSpec() {
val propsNull = ParticipantProperties(null)

val conferenceBridges = mutableMapOf<Bridge, ConferenceBridgeProperties>()
// 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

Expand Down Expand Up @@ -135,8 +136,9 @@ class BridgeSelectionStrategyTest : ShouldSpec() {

val conferenceBridges = mutableMapOf<Bridge, ConferenceBridgeProperties>()

// 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
Expand Down
Loading