fix: harden ghost client cleanup and PRE_ACTIVE state guard

- Change ChannelState.PRE_ACTIVE from list to frozenset (immutable,
  O(1) membership tests, no risk of silent mutation)

- Add optional client_ids parameter to ClientManager.remove_ghost_clients()
  so callers that have already fetched SMEMBERS can pass it through,
  eliminating a redundant Redis round-trip

- Pass pre-fetched client_ids from get_basic_channel_info() into
  remove_ghost_clients() to remove the duplicate SMEMBERS call
This commit is contained in:
SergeantPanda 2026-03-15 19:08:38 -05:00
parent c65ace1a7c
commit 2d2b3e45f4
3 changed files with 11 additions and 4 deletions

View file

@ -445,8 +445,9 @@ class ChannelStatus:
client_ids = proxy_server.redis_client.smembers(client_set_key)
# Remove ghost SET entries before building the client list.
# Pass the already-fetched client_ids to avoid a redundant SMEMBERS.
stale_client_ids = ClientManager.remove_ghost_clients(
proxy_server.redis_client, channel_id
proxy_server.redis_client, channel_id, client_ids=client_ids
)
if stale_client_ids:
client_count = max(0, client_count - len(stale_client_ids))

View file

@ -415,14 +415,20 @@ class ClientManager:
logger.error(f"Error refreshing client TTL: {e}")
@staticmethod
def remove_ghost_clients(redis_client, channel_id):
def remove_ghost_clients(redis_client, channel_id, client_ids=None):
"""Remove client SET entries whose metadata hash has expired.
Returns the list of removed (stale) client IDs, or an empty list
if none were found. Uses a pipelined EXISTS check for efficiency.
Args:
client_ids: Optional pre-fetched result of SMEMBERS for this
channel. Pass this to avoid a redundant SMEMBERS
call when the caller has already fetched it.
"""
client_set_key = RedisKeys.clients(channel_id)
client_ids = redis_client.smembers(client_set_key)
if client_ids is None:
client_ids = redis_client.smembers(client_set_key)
if not client_ids:
return []

View file

@ -22,7 +22,7 @@ class ChannelState:
# States before a channel is fully active. Used by the stream manager
# finally block to decide whether a failed stream can write ERROR.
PRE_ACTIVE = [INITIALIZING, CONNECTING, BUFFERING, WAITING_FOR_CLIENTS]
PRE_ACTIVE = frozenset([INITIALIZING, CONNECTING, BUFFERING, WAITING_FOR_CLIENTS])
# Event types
class EventType: