-
Notifications
You must be signed in to change notification settings - Fork 3.3k
improvment(sockets): migrate to redis #3072
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.
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
Greptile OverviewGreptile SummaryMigrated socket room management from in-memory to Redis-backed storage to enable multi-pod horizontal scaling. The implementation provides a clean abstraction with Key Changes
ArchitectureThe migration follows a factory pattern where
TestingTests were updated to use Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client Browser
participant SocketIO as Socket.IO Server
participant RoomMgr as Room Manager
participant Redis as Redis
participant Adapter as Redis Adapter
Note over Client,Redis: Connection Establishment
Client->>SocketIO: Connect with auth token
SocketIO->>RoomMgr: Initialize (createRoomManager)
alt Redis URL configured
RoomMgr->>Redis: Connect & load Lua scripts
Redis-->>RoomMgr: Connected
SocketIO->>Adapter: Configure Redis adapter
Note over Adapter: Enables cross-pod broadcasting
else No Redis URL
Note over RoomMgr: Use MemoryRoomManager
end
Note over Client,Redis: Join Workflow
Client->>SocketIO: emit('join-workflow', {workflowId, tabSessionId})
SocketIO->>RoomMgr: getWorkflowIdForSocket(socketId)
RoomMgr->>Redis: GET socket:{socketId}:workflow
Redis-->>RoomMgr: Current workflowId or null
alt User in different workflow
SocketIO->>RoomMgr: removeUserFromRoom(socketId)
RoomMgr->>Redis: EVALSHA removeUserScript
Note over Redis: Atomic: remove user,<br/>cleanup empty rooms
Redis-->>RoomMgr: Previous workflowId
end
SocketIO->>RoomMgr: getWorkflowUsers(workflowId)
RoomMgr->>Redis: HGETALL workflow:{wfId}:users
Redis-->>RoomMgr: Existing users list
Note over SocketIO: Check for stale connections<br/>from same user/tab
SocketIO->>RoomMgr: addUserToRoom(workflowId, socketId, presence)
RoomMgr->>Redis: MULTI pipeline
RoomMgr->>Redis: HSET workflow:{wfId}:users
RoomMgr->>Redis: SET socket:{socketId}:workflow
RoomMgr->>Redis: HSET socket:{socketId}:session
RoomMgr->>Redis: EXEC
Redis-->>RoomMgr: Success
SocketIO->>Client: emit('join-workflow-success')
SocketIO->>RoomMgr: broadcastPresenceUpdate(workflowId)
RoomMgr->>Adapter: emit to room (cross-pod)
Adapter->>Redis: PUBLISH to channel
Note over Redis,Adapter: All pods receive via Redis pub/sub
Adapter-->>SocketIO: Broadcast to local clients
SocketIO-->>Client: emit('presence-update')
Note over Client,Redis: Collaborative Updates
Client->>SocketIO: emit('cursor-update')
SocketIO->>RoomMgr: updateUserActivity(wfId, socketId, {cursor})
RoomMgr->>Redis: EVALSHA updateActivityScript
Note over Redis: Atomic read-modify-write
Redis-->>RoomMgr: Updated
SocketIO->>Adapter: emit to room (cross-pod)
Adapter->>Redis: PUBLISH to channel
Adapter-->>SocketIO: Broadcast to all pods
SocketIO-->>Client: emit('cursor-update')
Note over Client,Redis: Disconnect
Client->>SocketIO: disconnect
SocketIO->>RoomMgr: removeUserFromRoom(socketId)
RoomMgr->>Redis: EVALSHA removeUserScript
Note over Redis: Atomic: HDEL user,<br/>DEL socket keys,<br/>cleanup if room empty
Redis-->>RoomMgr: workflowId
SocketIO->>RoomMgr: broadcastPresenceUpdate(workflowId)
RoomMgr->>Adapter: emit to room (cross-pod)
Adapter-->>SocketIO: Update all pods
SocketIO-->>Client: emit('presence-update')
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 files reviewed, 3 comments
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Summary
Need to migrate to redis for sockets room membership to work correctly in multi-task context. Maintains in memory fallback for single-pod context.
Type of Change
Testing
Tested manually
Checklist