feat(be): implement participant presence in study rooms#3541
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a real-time study room feature using NestJS WebSockets, Redis, and BullMQ for session management. It introduces a StudyRoomService, a StudyGateway, and a global RedisModule. Feedback highlights a critical issue where modifying socket data via fetchSockets() fails to persist across clustered nodes, and identifies a bug where a code property is missing from join validation results. Further improvements are suggested to make the Redis database index configurable, remove the redundant redis dependency, and provide specific error codes for consistent client-side handling.
| useFactory: async (config: ConfigService) => { | ||
| const host = config.get<string>('REDIS_HOST') | ||
| const port = config.get<number>('REDIS_PORT') | ||
| const db = 1 |
There was a problem hiding this comment.
The Redis database index is hardcoded to 1. It is better to make this configurable via ConfigService to allow for different environments or shared Redis instances.
| const db = 1 | |
| const db = config.get<number>('REDIS_DB', 1) |
There was a problem hiding this comment.
이것도 뭐임! app.module.ts도 확인부탁
There was a problem hiding this comment.
admin 단에서 사용하던 Redis 설정을 그대로 가져온 코드였어서 확인해보니
이전에 BullMQ와 Cache 간 Redis DB 충돌로 장애가 발생해서 (#3069) DB를 분리한 이력이 있는데, 해당 설정을 그대로 차용했습니다.
현재 서비스 상 Redis DB 구조는 다음과 같은데
db 0 → CacheModule
db 1 → BullMQ (client: prefix 'bull-client', admin: prefix 'bull')
db 1 → RedisModule (웹소켓 어댑터, 스터디룸 기능)
BullMQ는 client/admin 각각 prefix로 키를 구분하고 있어서 같은 db 1을 사용해도 충돌은 없을 것 같습니다. 근데 충돌이 있을 수도 있어서 분리하는 것이 좋을 것 같기도 합니다.
또한 RedisModule(웹소켓 어댑터 + 스터디룸)은 prefix 없이 db 1을 그대로 사용하고 있어서 다른 db로 분리하는 게 안전할 것 같은데, 이 부분은 제가 임의로 결정하기보다 같이 논의하고 싶은데 어떻게 생각하시나요?
| "pino-http": "^10.5.0", | ||
| "pino-pretty": "^11.3.0", | ||
| "prisma-graphql-type-decimal": "^3.0.1", | ||
| "redis": "^5.11.0", |
There was a problem hiding this comment.
The redis package is added as a dependency, but the implementation uses ioredis (line 86). Unless another library specifically requires the redis package, it should be removed to keep dependencies clean.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a real-time study room system using WebSockets, Redis, and BullMQ, including a new Redis library and updates to the authentication guard. Feedback focused on improving robustness and maintainability by implementing safe JSON parsing for Redis data, standardizing user object property access, and making the Redis database index configurable.
| useFactory: async (config: ConfigService) => { | ||
| const host = config.get<string>('REDIS_HOST') | ||
| const port = config.get<number>('REDIS_PORT') | ||
| const db = 1 |
There was a problem hiding this comment.
The Redis database index is hardcoded to '1'. This might conflict with other services or environments that expect to use a different database or the default database '0'. Consider making the database index configurable via environment variables.
| const db = 1 | |
| const db = config.get<number>('REDIS_DB') ?? 1 |
| ) | ||
|
|
||
| const sockets = await this.server.in(roomKey(groupId)).fetchSockets() | ||
| await Promise.all(sockets.map((s) => s.leave(roomKey(groupId)))) |
There was a problem hiding this comment.
.leave()로 호출하면 같은 서버 인스턴스 소켓만 동작하고 다른 노드 소켓은 room에서 안나가요
disconnectSockets() 사용하는게 나을듯?
There was a problem hiding this comment.
궁금해서 저도 찾아봤는데요.
fetchSockets 메서드를 살펴보면 다음과 같이 나와요
Returns the matching socket instances. This method works across a cluster of several Socket.IO servers.
Note: this method also works within a cluster of multiple Socket.IO servers, with a compatible [Adapter]
저희는 redis-adapter를 사용해서 pub/sub 기능을 구현하니깐 다른 pod들의 소켓까지 leave 처리를 할 수 있는 것 같아요.
| useFactory: async (config: ConfigService) => { | ||
| const host = config.get<string>('REDIS_HOST') | ||
| const port = config.get<number>('REDIS_PORT') | ||
| const db = 1 |
There was a problem hiding this comment.
이것도 뭐임! app.module.ts도 확인부탁
| cors: { | ||
| // TODO 실제 서버 환경에 맞는 'cors' RedisIoAdapter 내부에 설정 | ||
| // origin: ['ws://localhost:3002/api/room'] | ||
| origin: true, |
| port: config.get<number>('REDIS_PORT'), | ||
| db: 1 | ||
| }, | ||
| prefix: 'bull-client' |
| ) | ||
|
|
||
| const sockets = await this.server.in(roomKey(groupId)).fetchSockets() | ||
| await Promise.all(sockets.map((s) => s.leave(roomKey(groupId)))) |
There was a problem hiding this comment.
궁금해서 저도 찾아봤는데요.
fetchSockets 메서드를 살펴보면 다음과 같이 나와요
Returns the matching socket instances. This method works across a cluster of several Socket.IO servers.
Note: this method also works within a cluster of multiple Socket.IO servers, with a compatible [Adapter]
저희는 redis-adapter를 사용해서 pub/sub 기능을 구현하니깐 다른 pod들의 소켓까지 leave 처리를 할 수 있는 것 같아요.
|
혹시 study-room 내부에 호스트를 별도로 지정한 이유가 있을까요? 예를 들면 공용 타이머는 |
|
그리고 아직 |
|
그리고 마지막으로 고생 많으셨다는 이야기를 드리고 싶어요. |
원래는 그룹 리더를 host로 두려고 했는데, 그룹 리더가 room에서 먼저 나가는 상황이 발생할 수 있다고 생각해서 이 부분은 정책에 따라 달라질 것 같은데, 그룹 리더가 나간 이후에도 room 내부 대표 참여자가 필요하다고 보면
네, 맞습니다. 현재 PR에서는 study room의 입장/퇴장/재연결/세션 종료 등 participant presence 처리에 집중했습니다. |
3 participants
Description
Study Room(Study Group 2depth)에서 사용자의 실시간 출입 상태 처리 기능을 구현합니다.
WebSocket(Socket.IO) + Redis + BullMQ 기반으로 입장/퇴장/재연결/세션 종료를 처리합니다.
Additional context
인프라 설정
@libs/redis: Redis 모듈 및RedisIoAdapter추가main.ts:RedisIoAdapter등록 (멀티 서버 환경 대응)JwtAuthGuard: WebSocket 컨텍스트에서handshake.auth.token으로 JWT 인증 처리study.module.ts:StudyGateway,StudyRoomService,StudyRoomProcessor등록구현
입장 (
room:join)SET NX로 race condition 방지 — 동시 입장 시 단 한 명만 첫 번째 입장자로 처리2시간 고정-> DB에서 endTime 가져오는 것으로 수정퇴장
room:leave): ack 응답으로 처리, 잔류 인원에게room:participantsChangedemithandleDisconnect): 30초 유예 후 완전 퇴장 처리reconnectKeyDEL 반환값으로 원자적 복구 판단 (TOCTOU 방지)세션 종료 (BullMQ)
room:reminderemitroom:endedemit → 소켓 강제 퇴장 → Redis 초기화room-end:{groupId})으로 중복 job 방지이벤트 목록
room:joinroom:leaveroom:startedroom:participantsChangedroom:participantReconnectingroom:participantReconnectedroom:reminderroom:endedBefore submitting the PR, please make sure you do the following
fixes #123).