Skip to content

fix: Handle a visitor node restarting. - #1307

Merged
bgrozev merged 1 commit into
masterfrom
handle-visitor-node-restart
Aug 25, 2026
Merged

fix: Handle a visitor node restarting.#1307
bgrozev merged 1 commit into
masterfrom
handle-visitor-node-restart

Conversation

@bgrozev

@bgrozev bgrozev commented Aug 19, 2026

Copy link
Copy Markdown
Member

When the XMPP stream to a visitor node is re-established without being resumed, the node has lost the state of the MUCs that jicofo joined on it. Smack does not re-join them, and MultiUserChat.isJoined() keeps returning true (it is only myRoomJid != null, which a connection loss does not clear). Jicofo therefore believed it was still in the visitor MUC, received no presence from it, and kept redirecting new visitors there. Only a jicofo restart recovered from this.

Changes

  • VisitorConnectionMonitor (new): one Smack ConnectionListener per visitor connection. It reports a reset on authenticated(resumed=false), i.e. when the state on the node is gone. A stream that resumes is left alone, which matches how the main room is handled.
  • FocusManager.visitorConnectionReset() notifies the conferences.
  • JitsiMeetConferenceImpl.visitorConnectionReset() discards the stale visitor room, terminates the visitors that were in it, leaves the room, and sends disconnect-vnode. The next redirect joins a new room.
  • selectVisitorNode() does not re-use a node whose XMPP connection is down. The fallback for when all nodes are down does not change.
  • selectVisitorNode() removes the map entry if the join fails, or if the room is discarded while we join it. Before this a failed join left an entry that jicofo treated as joined for the rest of the conference.

A visitor room that the server destroys had the same leak: the visitors in it kept a Participant and an endpoint on a bridge. Both paths now share the cleanup.

Tests

23 new tests in ConferenceUtilTest, ConferenceVisitorsTest and VisitorConnectionMonitorTest. With the changes to the production code reverted, 8 of them fail.

MockChatRoom.join() returned a relaxed mock, so mainRoomJid was never null and every conference in the tests looked like a breakout room. It now returns a real ChatRoomInfo.

When the XMPP stream to a visitor node is re-established without being
resumed, the node has lost the state of the MUCs that jicofo joined on it.
Smack does not re-join them and it keeps reporting them as joined, so jicofo
kept sending visitors to a room that it was not an occupant of anymore.

Detect this, then discard the visitor room, terminate the visitors that were
in it, and disconnect the node. Do not re-use a node whose XMPP connection is
down. Do not keep a room that we failed to join.
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.88%. Comparing base (7ebb13d) to head (143aac9).

Files with missing lines Patch % Lines
...tsi/jicofo/conference/JitsiMeetConferenceImpl.java 75.60% 8 Missing and 2 partials ⚠️
...o/src/main/kotlin/org/jitsi/jicofo/FocusManager.kt 0.00% 5 Missing ⚠️
.../main/kotlin/org/jitsi/jicofo/xmpp/XmppServices.kt 0.00% 3 Missing ⚠️
.../org/jitsi/jicofo/xmpp/VisitorConnectionMonitor.kt 88.88% 2 Missing ⚠️
...tlin/org/jitsi/jicofo/conference/ConferenceUtil.kt 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1307      +/-   ##
==========================================
+ Coverage   51.43%   52.88%   +1.44%     
==========================================
  Files         132      133       +1     
  Lines        8457     8511      +54     
  Branches     1290     1293       +3     
==========================================
+ Hits         4350     4501     +151     
+ Misses       3496     3384     -112     
- Partials      611      626      +15     
Files with missing lines Coverage Δ
...g/jitsi/jicofo/conference/JitsiMeetConference.java 0.00% <ø> (ø)
...tlin/org/jitsi/jicofo/conference/ConferenceUtil.kt 48.38% <66.66%> (+20.80%) ⬆️
.../org/jitsi/jicofo/xmpp/VisitorConnectionMonitor.kt 88.88% <88.88%> (ø)
.../main/kotlin/org/jitsi/jicofo/xmpp/XmppServices.kt 0.00% <0.00%> (ø)
...o/src/main/kotlin/org/jitsi/jicofo/FocusManager.kt 37.43% <0.00%> (-1.08%) ⬇️
...tsi/jicofo/conference/JitsiMeetConferenceImpl.java 47.35% <75.60%> (+9.20%) ⬆️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ebb13d...143aac9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jitsi-ci Bot pushed a commit to jitsi/jitsi-pr-tests-pages that referenced this pull request Aug 19, 2026
@bgrozev
bgrozev merged commit 9c814ab into master Aug 25, 2026
6 checks passed
@bgrozev
bgrozev deleted the handle-visitor-node-restart branch August 25, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants