From d8356e012e462adcec26878e93b4cb5f07d66eb2 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Thu, 23 Mar 2023 23:57:11 -0700 Subject: [PATCH] Give proper grace period when recorder is still in the room (#1547) When a recorder is in the room, we would skip grace period due to a bug with how LastLeftAt was set. This would cause RoomComposite templates to exit immediately if the last participant in the room reconnects. --- pkg/rtc/room.go | 30 +++++++++++++++++------------- pkg/rtc/room_test.go | 8 +++++--- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/pkg/rtc/room.go b/pkg/rtc/room.go index bd2f55fe4..ebdd590ef 100644 --- a/pkg/rtc/room.go +++ b/pkg/rtc/room.go @@ -27,14 +27,18 @@ import ( const ( DefaultEmptyTimeout = 5 * 60 // 5m - DefaultRoomDepartureGrace = 20 - AudioLevelQuantization = 8 // ideally power of 2 to minimize float decimal + AudioLevelQuantization = 8 // ideally power of 2 to minimize float decimal invAudioLevelQuantization = 1.0 / AudioLevelQuantization subscriberUpdateInterval = 3 * time.Second dataForwardLoadBalanceThreshold = 20 ) +var ( + // var to allow unit test override + RoomDepartureGrace uint32 = 20 +) + type broadcastOptions struct { skipSource bool immediate bool @@ -464,6 +468,11 @@ func (r *Room) RemoveParticipant(identity livekit.ParticipantIdentity, pID livek // send broadcast only if it's not already closed sendUpdates := !p.IsDisconnected() + // remove all published tracks + for _, t := range p.GetPublishedTracks() { + r.trackManager.RemoveTrack(t) + } + p.OnTrackUpdated(nil) p.OnTrackPublished(nil) p.OnTrackUnpublished(nil) @@ -476,11 +485,7 @@ func (r *Room) RemoveParticipant(identity livekit.ParticipantIdentity, pID livek r.Logger.Debugw("closing participant for removal", "pID", p.ID(), "participant", p.Identity()) _ = p.Close(true, reason) - r.lock.RLock() - if len(r.participants) == 0 { - r.leftAt.Store(time.Now().Unix()) - } - r.lock.RUnlock() + r.leftAt.Store(time.Now().Unix()) if sendUpdates { if r.onParticipantChanged != nil { @@ -597,16 +602,15 @@ func (r *Room) CloseIfEmpty() { } } - timeout := r.protoRoom.EmptyTimeout + var timeout uint32 var elapsed int64 - if r.FirstJoinedAt() > 0 { - // exit 20s after + if r.FirstJoinedAt() > 0 && r.LastLeftAt() > 0 { elapsed = time.Now().Unix() - r.LastLeftAt() - if timeout > DefaultRoomDepartureGrace { - timeout = DefaultRoomDepartureGrace - } + // need to give time in case participant is reconnecting + timeout = RoomDepartureGrace } else { elapsed = time.Now().Unix() - r.protoRoom.CreationTime + timeout = r.protoRoom.EmptyTimeout } r.lock.Unlock() diff --git a/pkg/rtc/room_test.go b/pkg/rtc/room_test.go index 2452eb6cc..9f4a4fd10 100644 --- a/pkg/rtc/room_test.go +++ b/pkg/rtc/room_test.go @@ -33,6 +33,8 @@ func init() { serverlogger.InitFromConfig(config.LoggingConfig{ Config: logger.Config{Level: "debug"}, }) + // allow immediate closure in testing + RoomDepartureGrace = 1 } var iceServersForRoom = []*livekit.ICEServer{{Urls: []string{"stun:stun.l.google.com:19302"}}} @@ -59,11 +61,11 @@ func TestJoinedState(t *testing.T) { require.LessOrEqual(t, s, rm.LastLeftAt()) }) - t.Run("LastLeftAt should not be set when there are still participants in the room", func(t *testing.T) { + t.Run("LastLeftAt should be set when there are still participants in the room", func(t *testing.T) { rm := newRoomWithParticipants(t, testRoomOpts{num: 2}) p0 := rm.GetParticipants()[0] rm.RemoveParticipant(p0.Identity(), p0.ID(), types.ParticipantCloseReasonClientRequestLeave) - require.EqualValues(t, 0, rm.LastLeftAt()) + require.Greater(t, rm.LastLeftAt(), int64(0)) }) } @@ -349,7 +351,7 @@ func TestRoomClosure(t *testing.T) { rm.protoRoom.EmptyTimeout = 0 rm.RemoveParticipant(p.Identity(), p.ID(), types.ParticipantCloseReasonClientRequestLeave) - time.Sleep(defaultDelay) + time.Sleep(time.Duration(RoomDepartureGrace)*time.Second + defaultDelay) rm.CloseIfEmpty() require.Len(t, rm.GetParticipants(), 0)