Skip to content
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

[BE] feat#219 redis 메모리 관리 구현 #236

Merged

Conversation

DongHoonYu96
Copy link
Collaborator

@DongHoonYu96 DongHoonYu96 commented Nov 20, 2024

➕ 이슈 번호


🔎 작업 내용

  • 새로운 기능과 모듈:
    스케줄링 기능 추가
    @nestjs/schedule 모듈 추가
    예약된 작업과 크론 작업 관리용
    이것을 이용, TTL이 설정되지 않은 데이터에 대해 TTL 설정

  • Redis 메모리 관리 서비스 추가
    GameRedisMemoryService 도입
    Redis 키의 TTL(Time To Live) 관리를 위한 스케줄 작업 처리

  • 방 정리 구독자 추가
    RoomCleanupSubscriber 구현
    Redis의 방 정리 이벤트 처리

  • 인터셉터:
    GameActivityInterceptor 도입
    방 활동 시간 업데이트 처리
    핵심 기능에서 비즈니스 로직 분리


🖼 참고 이미지

기존 test 통과
image

🎯 리뷰 요구사항 (선택)

  • test에 대한 생각이 어떠신가요
  • 회귀테스트(오류가 발생한부분에 대해서 test code 작성) 만 하는건 어떤지.. (사실 귀찮아서?)
    image

✅ Check List

  • merge할 브랜치의 위치를 확인했나요?
  • Label을 지정했나요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 19 changed files in this pull request and generated no suggestions.

Files not reviewed (14)
  • BE/package-lock.json: Language not supported
  • BE/package.json: Language not supported
  • BE/src/game/redis/subscribers/room.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/base.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/timer.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/redis-subscriber.service.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/scoring.subscriber.ts: Evaluated as low risk
  • BE/test/integration/game.integration.spec.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/player.subscriber.ts: Evaluated as low risk
  • BE/src/app.module.ts: Evaluated as low risk
  • BE/src/game/service/quiz.cache.service.ts: Evaluated as low risk
  • BE/src/game/game.module.ts: Evaluated as low risk
  • BE/src/game/game.gateway.ts: Evaluated as low risk
  • BE/src/game/service/game.service.ts: Evaluated as low risk
@DongHoonYu96 DongHoonYu96 requested a review from Copilot November 20, 2024 15:11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 19 changed files in this pull request and generated no suggestions.

Files not reviewed (14)
  • BE/package-lock.json: Language not supported
  • BE/package.json: Language not supported
  • BE/src/game/redis/subscribers/player.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/timer.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/scoring.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/room.cleanup.subscriber.ts: Evaluated as low risk
  • BE/src/game/service/game.service.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/room.subscriber.ts: Evaluated as low risk
  • BE/src/game/redis/redis-subscriber.service.ts: Evaluated as low risk
  • BE/src/game/redis/subscribers/base.subscriber.ts: Evaluated as low risk
  • BE/test/integration/game.integration.spec.ts: Evaluated as low risk
  • BE/src/app.module.ts: Evaluated as low risk
  • BE/src/game/game.module.ts: Evaluated as low risk
  • BE/src/game/service/quiz.cache.service.ts: Evaluated as low risk
방장이 잠수인경우, 명시적 퇴장이 없음
Copy link
Collaborator

@NewCodes7 NewCodes7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동훈님 수고하셨습니다!! 메모리 관리 좋네요 ㅎㅎ

const data = context.switchToWs().getData();
if (data.gameId) {
await this.gameRoomService.updateRoomActivity(data.gameId);
this.logger.debug(`Activity updated for room ${data.gameId} after ${Date.now() - before}ms`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: 지금 로그가 한글로 찍혀 있는 부분이 많아서 한글로 통일하면 좋을 것 같습니다!

@@ -0,0 +1,28 @@
// 활동 시간 업데이트는 비즈니스 로직과 분리
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍 좋습니다!

Comment on lines +51 to +57
/**
* 배치 단위로 TTL 설정
* Pipeline 사용으로 네트워크 요청 최소화
*/
private async processBatch(rooms: string[]): Promise<void> {
const pipeline = this.redis.pipeline();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 pipeline이란 게 있군요! 좋습니다!

Comment on lines +7 to +17
@Injectable()
export class GameRedisMemoryService {
private readonly logger = new Logger(GameRedisMemoryService.name);
private readonly BATCH_SIZE = 100; // 한 번에 처리할 배치 크기

private readonly TTL = {
ROOM: 3 * 60 * 60,
PLAYER: 2 * 60 * 60,
QUIZ: 1 * 60 * 60,
LEADERBOARD: 3 * 60 * 60
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 클래스는 누군가 레디스에 데이터를 집어넣으면, 해당 데이터에 만료기간을 추가로 달아주는 게 주 역할인 걸로 이해했는데 맞을까요? 레디스에 게임방 관련 데이터를 집어넣을 때부터 만료기간을 정해서 넣어주는 방법도 있었을 것 같은데, 이렇게 따로 서비스를 만들어주신 게 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TTL 설정을 여기서 담당하는것도 나쁘지않겠다.
TTL이 이상하다면 이 클래스만 보면됨.
단점 : 성능상은 안좋을듯함

Copy link
Collaborator

@ijun17 ijun17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! LGTM

Copy link
Collaborator

@songbuild00 songbuild00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 👍

Comment on lines +9 to +11
// 전역 인터셉터로 등록
app.useGlobalInterceptors(app.get(GameActivityInterceptor));

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GameGateway에 달린 @UseInterceptors(GameActivityInterceptor) 와는 다른 것인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 해야 인터셉터가 작동을 하는것 같습니다

@DongHoonYu96 DongHoonYu96 merged commit 57d5438 into boostcampwm-2024:dev-be Nov 21, 2024
@DongHoonYu96 DongHoonYu96 deleted the feature-be-#219#229 branch November 21, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants