Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ class IceRestartMetrics {
"Number of in-place ICE restarts for which the bridge's rotated transport was signaled to the participant."
)

/**
* The bridge answered with the transport it already had, because it did not need to restart (its own
* transport has not connected yet). Nothing was relayed and the participant keeps the session it has, so
* this is neither a completed restart nor a failed one.
*/
@JvmField
val notNeeded = metricsContainer.registerCounter(
"ice_restarts_not_needed",
"Number of in-place ICE restarts for which the bridge kept its existing transport."
)

/**
* An ICE restart did not complete: the request was rejected (disabled, rate-limited, stale bridge-session ID)
* or the bridge's response could not be relayed (no transport in the response, stale generation, participant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,11 @@ class ColibriV2SessionManager @JvmOverloads constructor(
* if the participant restarts twice in quick succession the two responses can arrive in either order. The
* `ice-generation` the bridge echoes tells us which round each belongs to, and anything older than what we
* have already relayed is discarded.
*
* A transport with no `ice-generation` at all means something different: the bridge did not restart and
* answered with the credentials of the Agent it already has. It does this when its transport has not
* connected yet, where there is nothing to preserve and nothing for the participant to do. There is nothing
* to relay in that case, and no restart to record.
*/
internal fun endpointIceRestarted(endpointId: String, transport: IceUdpTransportPacketExtension) {
val generation = transport.iceGeneration
Expand All @@ -802,8 +807,12 @@ class ColibriV2SessionManager @JvmOverloads constructor(
IceRestartMetrics.failed.inc()
return
}
if (generation != IceUdpTransportPacketExtension.GENERATION_UNSPECIFIED &&
participantInfo.lastRelayedIceGeneration != IceUdpTransportPacketExtension.GENERATION_UNSPECIFIED &&
if (generation == IceUdpTransportPacketExtension.GENERATION_UNSPECIFIED) {
logger.info("ICE restart: the bridge kept its existing transport for $endpointId, not relaying.")
IceRestartMetrics.notNeeded.inc()
return
}
if (participantInfo.lastRelayedIceGeneration != IceUdpTransportPacketExtension.GENERATION_UNSPECIFIED &&
generation <= participantInfo.lastRelayedIceGeneration
) {
logger.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import org.jitsi.utils.time.FakeClock
import org.jitsi.xmpp.extensions.colibri2.ConferenceModifyIQ
import org.jitsi.xmpp.extensions.jingle.DtlsFingerprintPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension.GENERATION_UNSPECIFIED
import org.jivesoftware.smack.packet.IQ
import org.jxmpp.jid.Jid
import org.jxmpp.jid.impl.JidCreate
Expand Down Expand Up @@ -359,15 +358,18 @@ class ColibriV2SessionManagerTest : ShouldSpec() {
}
}

context("With a bridge that does not tag generations") {
sessionManager.endpointIceRestarted("p1", transportWithGeneration(null)).also { drain() }
context("With a transport that carries no generation") {
// The bridge kept the Agent it already had instead of restarting, so there is nothing to relay.
sessionManager.endpointIceRestarted("p1", transportWithGeneration(2)).also { drain() }
sessionManager.endpointIceRestarted("p1", transportWithGeneration(null)).also { drain() }

should("relay everything (the guard can not order untagged transports)") {
iceRestartedTransports.size shouldBe 2
iceRestartedTransports.map {
it.second.iceGeneration
} shouldBe listOf(GENERATION_UNSPECIFIED, GENERATION_UNSPECIFIED)
should("not relay it") {
iceRestartedTransports.map { it.second.iceGeneration } shouldBe listOf(2)
}
should("not lose the generation of the last transport that was relayed") {
sessionManager.endpointIceRestarted("p1", transportWithGeneration(1)).also { drain() }

iceRestartedTransports.map { it.second.iceGeneration } shouldBe listOf(2)
}
}
}
Expand Down
Loading