Skip to content

Feat: 비활성 채팅방 자동 종료 스케쥴러 구현#114

Merged
CheatIsKey merged 43 commits into
developfrom
feat/Chat-Inactivity-Scheduler#113
Apr 24, 2026
Merged

Feat: 비활성 채팅방 자동 종료 스케쥴러 구현#114
CheatIsKey merged 43 commits into
developfrom
feat/Chat-Inactivity-Scheduler#113

Conversation

@CheatIsKey
Copy link
Copy Markdown
Member

💡 개요

  • 비활성 채팅방 자동 종료 스케쥴러 구현

🛠️ 작업 내용

ChatInactivityScheduler 추가

  • 1분 주기로 비활성 채팅방 감지 및 자동 종료 처리
  • 기준 시간(inactivity-timeout-minutes) application.yml로 외부화
  • 종료 대상: lastMessageAt 기준 설정 시간 경과 + WAITING/IN_PROGRESS 상태
  • 개별 try-catch 적용으로 단일 방 처리 실패 시 나머지 방 계속 처리
  • TODO: 분산락 적용 예정 (현재 비관적 락 + 상태 재확인으로 1차 방어)

autoCloseRoom() 추가

  • 스케쥴러 전용 종료 메서드 분리 (closeChatRoom과 역할 분리)
  • findByIdForUpdate() 비관적 락으로 동시 상태 변경 방어
  • 이미 COMPLETED 상태면 스킵 (분산 환경 중복 실행 방어)
  • 시스템 메시지 DB 저장 후 Redis Pub/Sub으로 클라이언트에 실시간 알림

findInactiveRooms() QueryDSL로 구현

  • JPQL 전체 패키지명 방식 제거 → ChatRoomRepositoryCustomImpl에 QueryDSL로 통합

  • chatRoom.chatRoomStatus.in(WAITING, IN_PROGRESS) + lastMessageAt.before(cutoff)

  • 기존 ChatRoomRepositoryCustom 구조에 맞춰 일관성 유지

  • Closes: [Feat] 실시간 채팅 자동 종료 스케쥴러 #113

ChatInactivityScheduler 추가
- 1분 주기로 비활성 채팅방 감지 및 자동 종료 처리
- 기준 시간(inactivity-timeout-minutes) application.yml로 외부화
- 종료 대상: lastMessageAt 기준 설정 시간 경과 + WAITING/IN_PROGRESS 상태
- 개별 try-catch 적용으로 단일 방 처리 실패 시 나머지 방 계속 처리
- TODO: 분산락 적용 예정 (현재 비관적 락 + 상태 재확인으로 1차 방어)

autoCloseRoom() 추가
- 스케쥴러 전용 종료 메서드 분리 (closeChatRoom과 역할 분리)
- findByIdForUpdate() 비관적 락으로 동시 상태 변경 방어
- 이미 COMPLETED 상태면 스킵 (분산 환경 중복 실행 방어)
- 시스템 메시지 DB 저장 후 Redis Pub/Sub으로 클라이언트에 실시간 알림

findInactiveRooms() QueryDSL로 구현
- JPQL 전체 패키지명 방식 제거 → ChatRoomRepositoryCustomImpl에 QueryDSL로 통합
- chatRoom.chatRoomStatus.in(WAITING, IN_PROGRESS) + lastMessageAt.before(cutoff)
- 기존 ChatRoomRepositoryCustom 구조에 맞춰 일관성 유지
@CheatIsKey CheatIsKey self-assigned this Apr 24, 2026
@CheatIsKey CheatIsKey added the enhancement New feature or request label Apr 24, 2026
Copy link
Copy Markdown
Contributor

@DGAZA-max DGAZA-max left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!!

@github-actions
Copy link
Copy Markdown

🤖 AI 코드리뷰

리더보드 — 현재 5위 (리뷰 91건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

🟡 개선 제안

1. 컴파일 에러 및 로깅 포맷 오류 (치명적)

log.info("[자동종료] 처리 대상 {}건");

인자를 1개 요구하고 있는데 아무것도 안 넘겼습니다. 빌드가 안 되거나 런타임에 MissingFormatArgumentException이 발생합니다. 즉시 targets.size()를 추가하세요.

2. 매직 스트링 하드코딩
알림 메시지인 "10분간 응답이 없어 상담이 자동 종료되었습니다."가 스케쥴러와 엔티티/도메인 로직 양쪽에 중복으로 하드코딩되어 있습니다. 타임아웃 시간(inactivityTimeoutMinutes)이 설정 파일에 따라 가변적임에도 메시지는 "10분"으로 박혀 있습니다.
메시지는 상수로 추출하거나, String.format 등을 사용해 동적으로 생성해야 합니다.

3. 로깅 순서로 인한 데이터 정합성 오류 (상태 변경 후 로깅)

chatRoom.changeStatus(ChatRoomStatus.COMPLETED);
// ...
log.info("[자동종료] 완료 roomId: {}, 이전 상태: {}", roomId, chatRoom.getChatRoomStatus());

이전 상태를 로깅하고 싶다면, 상태를 변경하기 에 로컬 변수에 담아두고 로깅해야 합니다. 현재 코드는 상태를 이미 COMPLETED로 바꾼 뒤에 로그를 찍고 있어 "이전 상태: COMPLETED"로 찍힙니다.

🔴 보안 / 성능 주의

1. 무페이징 대량 조회 (Full Table Scan 위험)

List<ChatRoom> findInactiveRooms(LocalDateTime cutoff)

fetch()로 조건에 맞는 모든 데이터를 메모리에 올립니다. 서비스 초기에는 문제없겠지만, 운영 환경에서 비활성 방이 10만 개가 넘어가는 순간 **OutOfMemory(OOM)**가 발생하며 서버가 죽습니다.
@Scheduled가 돌 때마다 터질 수 있는 시한폭탄입니다. 대량의 데이터를 다룰 때는 반드시 페이징 처리(Pageable)하거나 ID 페이징(No-Offset) 방식을 도입해야 합니다.

💡 학습 포인트

분산 환경에서의 스케쥴러 동시성 제어
주석에 "현재는 비관적 락으로 방어"라고 적어놓으셨습니다. 방어가 아예 안 되고 있는 것은 아니지만, 비관적 락은 DB 커넥션을 물고 있는 시간만큼의 병목을 유발할 뿐 "분산 환경에서의 동시 실행" 자체를 막아주진 않습니다.
주석에 남기신 "LockService 완성 후 분산락(Lettuce 기반) 적용 예정" 방향이 맞습니다. 스케쥴러 진입 자체를 Lock으로 통제하지 않으면 다중 서버 환경에서 불필요한 DB I/O와 커넥션 풀 고갈이 발생합니다.

🤔 생각해보기

이 스케쥴러가 1분 주기로 돌고 있고, autoCloseRoom()에서 비관적 락(SELECT ... FOR UPDATE)을 획득하기 위해 DB 커넥션을 점유하고 있다고 가정해 봅시다. 비활성 채팅방이 1만 개여서 루프를 도는 데 5분이 걸리게 된다면, 이 서버의 커넥션 풀과 스케쥴러 스레드 풀에는 어떤 연쇄적인 장애가 발생할까요?

💬 이 질문에 대해 궁금한 점이 있으면 코멘트에 @sparta 를 남겨보세요!


AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.

ChatInactivityScheduler 추가
- 1분 주기로 비활성 채팅방 감지 및 자동 종료 처리
- 기준 시간(inactivity-timeout-minutes) application.yml로 외부화
- 종료 대상: lastMessageAt 기준 설정 시간 경과 + WAITING/IN_PROGRESS 상태
- 개별 try-catch 적용으로 단일 방 처리 실패 시 나머지 방 계속 처리
- TODO: 분산락 적용 예정 (현재 비관적 락 + 상태 재확인으로 1차 방어)

autoCloseRoom() 추가
- 스케쥴러 전용 종료 메서드 분리 (closeChatRoom과 역할 분리)
- findByIdForUpdate() 비관적 락으로 동시 상태 변경 방어
- 이미 COMPLETED 상태면 스킵 (분산 환경 중복 실행 방어)
- 시스템 메시지 DB 저장 후 Redis Pub/Sub으로 클라이언트에 실시간 알림

findInactiveRooms() QueryDSL로 구현
- JPQL 전체 패키지명 방식 제거 → ChatRoomRepositoryCustomImpl에 QueryDSL로 통합
- chatRoom.chatRoomStatus.in(WAITING, IN_PROGRESS) + lastMessageAt.before(cutoff)
- 기존 ChatRoomRepositoryCustom 구조에 맞춰 일관성 유지
@github-actions
Copy link
Copy Markdown

🤖 AI 코드리뷰

리더보드 — 현재 5위 (리뷰 92건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

📋 과제 요구사항 준수 여부

해당 PR은 도전 기능 중 실시간 채팅(비활성 채팅방 자동 종료) 확장 기능에 해당합니다.

  • Lettuce 기반 Redis Lock: RedisLockRepository를 통해 tryLock / unlock을 구현하고 있으며, 과제에서 금지하는 Redisson을 사용하지 않아 양호합니다.
  • 동시성 제어: 분산락(Lettuce) + DB 비관적 락(Pessimistic Lock)을 혼합하여 사용하고 있어 다중 서버 환경에서의 동시성 이슈를 방어하는 구조입니다.

🟢 잘된 점

1. 이전 피드백(치명적 오류) 완벽하게 반영
이전 리뷰에서 지적된 3가지 핵심 문제(컴파일 에러, 매직 스트링, 로깅 오류)가 모두 정확하게 수정되었습니다. 특히 상태 변경 전 로컬 변수(previousStatus)로 값을 백업하여 로깅하는 것은 데이터 정합성 측면에서 아주 훌륭한 접근입니다.

2. 심도 있는 동시성 제어 설계
다중 인스턴스 환경에서 같은 스케쥴러가 동시에 도는 것을 Lettuce 분산락으로 1차 방어하고, 개별 채팅방 종료 시에는 DB 비관적 락(findByIdForUpdate)으로 2차 방어하는 이중 안전장치 구조가 매우 인상적입니다. 실무적인 접근법입니다.

3. 장애 격리(Fault Isolation) 처리
for 루프 내에서 각 채팅방 처리 로직을 개별 try-catch로 감싼 점이 좋습니다. 수백 개의 방 중 1개에서 예외가 발생해도 나머지 방의 종료 처리는 정상적으로 진행되어 전체 서비스가 멈추는 사태를 막을 수 있습니다.

🔴 필수 수정

특이사항 없음. 이전에 지적된 컴파일 에러 및 로직 오류가 모두 해결되었습니다.

🟡 권장 개선

1. 여전히 잠재적인 OOM(Out Of Memory) 위험 존재
이전 리뷰에서 지적된 대량 조회(List<ChatRoom>) 이슈가 여전히 남아있습니다. 현재 서비스 규모가 작아 문제가 없겠지만, 비활성 방이 10만 개 이상 쌓이는 순간 스케쥴러가 메모리를 모두 점유해 서버가 다운될 수 있습니다.

해결 방법 (ID 페이징 / No-Offset 방식 도입)
루프를 돌며 일정 배치(Batch) 사이즈만큼만 조회하도록 변경하는 것을 권장합니다.

// ChatRoomRepositoryCustom.java
public interface ChatRoomRepositoryCustom {
-    List<ChatRoom> findInactiveRooms(LocalDateTime cutoff);
+    List<ChatRoom> findInactiveRooms(LocalDateTime cutoff, Long lastId, int batchSize);
}

// ChatRoomRepositoryCustomImpl.java
+import com.querydsl.core.types.dsl.BooleanExpression;

public class ChatRoomRepositoryCustomImpl implements ChatRoomRepositoryCustom {
     @Override
-    public List<ChatRoom> findInactiveRooms(LocalDateTime cutoff) {
+    public List<ChatRoom> findInactiveRooms(LocalDateTime cutoff, Long lastId, int batchSize) {
         return queryFactory
                 .selectFrom(chatRoom)
                 .where(
+                        gtRoomId(lastId),
                         chatRoom.chatRoomStatus.in(
                                 ChatRoomStatus.WAITING,
                                 ChatRoomStatus.IN_PROGRESS
                         ),
                         chatRoom.lastMessageAt.before(cutoff)
-                ).fetch();
+                )
+                .orderBy(chatRoom.id.asc())
+                .limit(batchSize)
+                .fetch();
     }

+    private BooleanExpression gtRoomId(Long lastId) {
+        return lastId != null ? chatRoom.id.gt(lastId) : null;
+    }
}

// ChatInactivityScheduler.java
-List<ChatRoom> targets = chatRoomRepository.findInactiveRooms(cutOff);
-// ... for 루프 ...

💡 학습 포인트

스케쥴러의 대용량 데이터 처리 패턴 (ID 페이징)
스케쥴러나 배치(Batch) 프로그램을 구현할 때는 select * from table where ... 형태의 무페이징 조회는 지양해야 합니다. 일반적인 웹 페이징(offset, limit)도 데이터가 많아지면 성능이 저하되므로, 마지막으로 조회한 ID를 기준으로 다음 데이터를 가져오는 No-Offset (Keyset Pagination) 방식을 학습해 보시면 좋습니다.

🤔 생각해보기

현재 분산락(Lettuce)을 적용할 때 tryLock 실패 시 return으로 아예 실행을 포기(Fail-Fast)하고 있습니다. 만약 Write-back 스케쥴러나 자정 초기화 스케쥴러가 실행되는 도중에 락의 TTL(55분)이 만료되어 버리면, 다른 서버가 중복으로 스케쥴러를 실행하게 됩니다. 스케쥴러의 실행 시간이 락의 TTL보다 길어질 위험이 있는 상황에서는, 어떤 전략을 통해 데이터의 중복 처리나 유실을 방지할 수 있을까요?

💬 이 질문에 대해 궁금한 점이 있으면 코멘트에 @sparta 를 남겨보세요!
예: @sparta 락 연장(Renewal)은 Lettuce 환경에서 어떻게 구현하나요?

🔄 이전 피드백 반영 여부

  • 컴파일 에러 수정: ✅ targets.size()를 인자로 넘기도록 정상 반영됨.
  • 매직 스트링 개선: ✅ 하드코딩된 "10분" 문자열이 inactivityTimeoutMinutes 설정값과 분리되어 하드코딩 이슈가 해결됨.
  • 로깅 오류 수정: ✅ 상태 변경 전 previousStatus 변수를 활용해 정확한 이전 상태를 로깅하도록 수정됨.
  • 무페이징 대량 조회: ❌ 미반영. 여전히 fetch()로 전체 데이터를 메모리에 올리는 구조입니다. (위 권장 개선 사항 참고)

AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.

@github-actions
Copy link
Copy Markdown

🤖 AI 코드리뷰

리더보드 — 현재 5위 (리뷰 93건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

🔄 이전 피드백 반영 여부

  • 컴파일 에러 / 매직 스트링 / 로깅 오류: ✅ 해결. 이전에 지적된 3가지 기본 오류는 모두 정상적으로 수정되었습니다.
  • 무페이징 대량 조회 (OOM 위험): ✅ 해결. 이전 리뷰에서 권장한 ID 페이징(No-Offset) 방식이 정확하게 반영되었습니다. lastIdbatchSize를 활용한 무한 루프 구조로 대량 데이터 환경에서의 메모리 고갈 위험을 제거한 점이 매우 훌륭합니다.

🟡 개선 제안

1. 매직 리터럴 상수화 미비
스케쥴러의 실행 주기(60_000)와 메시지 하드코딩("10분간 응답이 없어...")이 여전히 클래스 내에 매직 리터럴로 흩어져 있습니다.
fixedRate@Scheduled 속성상 어쩔 수 없다 쳐도, 알림 메시지는 최소한 상단에 private static final String INACTIVE_CLOSE_MESSAGE = "..."; 처럼 상수로 추출하거나, 동적으로 생성하도록 개선해야 합니다. 6개월 뒤에 타임아웃 시간을 30분으로 바꿀 때 이 메시지도 같이 찾아서 고칠 것이라고 보장할 수 있습니까?

2. 대량 트랜잭션 병목 (N+1 쓰기 문제)
현재 구조는 배치로 조회한 뒤, for 루프를 돌며 건별로 autoCloseRoom()을 호출하고 있습니다. 내부 로직을 보면 findByIdForUpdate (비관적 락 SELECT) + 상태 변경 + 메시지 INSERT가 각 방마다 트랜잭션 단위로 개별 실행됩니다.
비활성 방이 1,000개만 넘어가도 DB 커넥션을 1,000번 물고 놓는 쓰기 폭풍이 발생합니다. 건별 처리가 꼭 필요한 상태라면 루프 내에서 개별 트랜잭션 경계를 명확히 하거나, 상태 변경 범위를 ID 리스트(IN 쿼리)로 묶어 한 번에 업데이트하는 Bulk Update 패턴을 고려해야 합니다.

🔴 보안 / 성능 주의

1. 락(Lock) 획득 실패 시 무한 대기 가능성 (장애 전파 위험)

if (!redisLockRepository.tryLock(WRITE_BACK_LOCK_KEY, lockValue, WRITE_BACK_LOCK_TTL)) {
    // ...
}

KeywordSchedulerwriteBack() 메서드는 @Scheduled(fixedDelay = ...)를 사용하고 있습니다. fixedDelay이전 작업이 완전히 끝난 후 지정된 시간만큼 쉬었다가 다시 실행하는 방식입니다.
현재 분산락 획득에 실패하면 즉시 return으로 메서드를 빠져나옵니다. 이때문에 fixedDelay의 카운터가 바로 0으로 리셋되어, 락을 잡고 있는 다른 서버의 작업이 끝나기도 전에 다시 락 획득을 시도하는 루프에 빠지게 됩니다. 서버가 2대일 때는 이 정도가 괜찮을지 모르겠지만, 서버가 N대로 늘어나고 락 점유 시간이 길어지면 불필요한 Redis I/O가 급증하게 됩니다.
실패 시 일정 시간 Thread를 대기(sleep)시키거나, fixedRate 대신 Cron 표현식을 고려하는 것이 운영 환경에서는 더 안전합니다.

🤔 생각해보기

현재 비활성 채팅방을 종료할 때, autoCloseRoom()에서 DB 비관적 락(SELECT ... FOR UPDATE)을 잡고 상태를 COMPLETED로 변경한 뒤 트랜잭션을 커밋합니다. 그 직후에 chatRedisPublisher.publish()를 통해 WebSocket 알림을 발행합니다.
만약 이 Redis Pub/Sub 발행 과정에서 네트워크 장애나 Redis 서버 다운으로 예외가 발생한다면 어떤 일이 벌어질까요? 클라이언트는 영영 채팅방이 종료되었다는 사실을 모르게 됩니다. 이 상황을 어떻게 방어할 수 있을까요?

💬 이 질문에 대해 궁금한 점이 있으면 코멘트에 @sparta 를 남겨보세요!


AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.

Copy link
Copy Markdown
Collaborator

@Max-1012 Max-1012 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Copy Markdown

🤖 AI 코드리뷰

리더보드 — 현재 5위 (리뷰 94건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

📋 과제 요구사항 준수 여부

  • Lettuce 기반 Redis Lock: RedisLockRepository를 통해 분산락을 적용하고 있으며, 과제에서 금지하는 Redisson을 사용하지 않아 양호합니다.
  • 실시간 채팅(도전 기능): 비활성 채팅방 자동 종료 스케쥴러 구현, WebSocket 테스트 페이지 제공 등 도전 기능의 요구사항을 충실히 반영하고 있습니다.

🟢 잘된 점

1. 이전 피드백 완벽 반영 및 아키텍처 대폭 개선
지적받았던 OOM 위험을 ID 페이징(No-Offset)으로 해결한 것은 물론, 건별 업데이트로 인한 DB 커넥션 고갈 이슈까지 **Bulk Update (IN 쿼리 활용)**로 완벽하게 해결하셨습니다. 단순히 오류만 고치는 수준을 넘어, 실무에서 사용하는 배치(Batch) 처리 아키텍처로 전체 구조를 개선한 점이 매우 인상적입니다.

2. 완벽한 장애 격리(Fault Isolation)와 graceful degradation
DB 커밋(상태 변경)과 Redis 알림(Pub/Sub)을 분리한 설계가 훌륭합니다. Redis 발행 실패 시에도 try-catch로 감싸 로그만 남기고 프로세스를 이어가며, 클라이언트는 재연결 시 DB 상태를 통해 변경을 감지할 수 있도록 한 주석의 설명(2차 감지)은 시니어 개발자다운 훌륭한 방어적 설계입니다.

3. KeywordScheduler의 분산락 도입
기존 ChatInactivityScheduler에만 적용되었던 분산락(Lettuce)을 KeywordScheduler에도 일관되게 적용하여 다중 서버 환경에서의 동시성 제어를 완벽히 갖추었습니다.

🔴 필수 수정

특이사항 없음. 이전에 지적된 필수 수정 사항 및 런타임 오류 위험이 모두 해결되었습니다.

🟡 권장 개선

1. autoCloseRoom 메서드의 잔여 정리
기존에 건별로 방을 종료하던 autoCloseRoom 메서드가 서비스 계층에 아직 남아있습니다. 현재 스케쥴러는 Bulk Update를 사용하므로 이 메서드를 사용하지 않는다면, 혼란을 방지하기 위해 제거하거나 @Deprecated 처리를 권장합니다.

2. Bulk Update 시 영속성 컨텍스트(Persistence Context) 불일치 주의
QueryDSL의 update().execute()는 영속성 컨텍스트를 거치지 않고 DB에 직접 쿼리를 날립니다. 직후 saveAll()로 시스템 메시지를 저장하므로 현재 로직상 문제는 없지만, 만약 Bulk Update 이후에 엔티티의 상태를 다시 조회하는 로직이 추가된다면 1차 캐시와 DB 간의 데이터 불일치가 발생할 수 있습니다. (참고: @Modifying(clearAutomatically = true))

💡 학습 포인트

  • 커서 기반 페이징 (Cursor-based Pagination / No-Offset): lastIdORDER BY id ASC를 활용하여 대용량 데이터에서도 일정한 성능을 내는 페이징 기법.
  • Write-behind / Bulk Update 패턴: 여러 번의 DB 쓰기를 하나의 배치 쿼리로 묶어 DB 커넥션과 네트워크 I/O를 최적화하는 기법.

🤔 생각해보기

현재 closeInactiveRoomswhile (true) 루프를 돌며 BATCH_SIZE(100) 단위로 데이터를 처리합니다. 만약 비활성 채팅방이 평소보다 월등히 많은 10만 개가 한 번에 쌓여 있다면, 이 스케쥴러는 1,000번의 루프를 돌며 Bulk Update를 수행하게 됩니다. 이때 스케쥴러의 전체 실행 시간이 1분(다음 스케쥴러 실행 주기)을 훌쩍 넘기게 된다면, 현재의 분산락(Lock TTL 55초) 구조에서는 어떤 문제가 발생할까요? 대용량 배치 처리 시 락(Lock)의 만료 시간과 작업 소요 시간 간의 충돌을 어떻게 방어할 수 있을까요?

💬 이 질문에 대해 궁금한 점이 있으면 코멘트에 @sparta 를 남겨보세요!
예: @sparta 스케쥴러에서 락 연장(Lock Extension)은 어떤 방식으로 구현하나요?

🔄 이전 피드백 반영 여부

  • 컴파일 에러 / 매직 스트링 / 로깅 오류: ✅ 완벽 반영됨. 이전에 지적된 기본 오류들은 깔끔하게 수정되었습니다.
  • 무페이징 대량 조회 (OOM 위험): ✅ 완벽 반영됨. lastId를 활용한 No-Offset 방식으로 OOM 위험이 완전히 제거되었습니다.
  • 건별 업데이트로 인한 DB 커넥션 고갈 (N+1 쓰기): ✅ 완벽 반영됨. bulkCompleteRooms()라는 Bulk Update 아키텍처를 도입하여 DB I/O를 극적으로 최적화했습니다.
  • KeywordScheduler 동시성 제어 미흡: ✅ 완벽 반영됨. writeBack, midnightReset 두 스케쥴러 모두 Lettuce 분산락이 추가되어 다중 서버 환경에서의 안정성이 확보되었습니다.

AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.

@github-actions
Copy link
Copy Markdown

🤖 AI 코드리뷰

리더보드 — 현재 5위 (리뷰 95건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

🔄 이전 피드백 반영 여부

  • 무페이징 대량 조회 (OOM 위험): ✅ 해결. lastIdbatchSize를 활용한 No-Offset 방식이 정확하게 반영되었습니다.
  • 건별 업데이트 (DB 커넥션 고갈): ✅ 해결. bulkCompleteRooms() 메서드를 통해 IN 쿼리 기반 Bulk Update로 트랜잭션 병목을 완화했습니다.
  • KeywordScheduler 동시성 제어: ✅ 해결. writeBack, midnightReset 메서드 모두 Lettuce 분산락이 추가되었습니다.
  • KeywordScheduler fixedDelay 이슈: ✅ 해결. fixedDelay를 Cron 표현식(0 0 * * * *)으로 변경하여 락 획득 실패 시 불필요한 Redis I/O 루프에 빠지는 문제를 방지했습니다.
  • 영속성 컨텍스트 불일치 주의: ✅ 해결. Bulk Update 직후 em.clear()를 명시적으로 호출하여 1차 캐시와 DB 간의 데이터 불일치 문제를 원천 차단했습니다. 아주 훌륭한 조치입니다.

🟡 개선 제안

1. @Scheduled와 트랜잭션 경계 분리
ChatInactivityScheduler.closeInactiveRooms()는 읽기, 수정, 쓰기, 외부 시스템(Redis) 발행을 단일 메서드에서 처리하고 있습니다. 현재 구조상 명시적인 @Transactional이 없으므로, bulkCompleteRooms() (JPQL update)와 chatMessageRepository.saveAll()이 각각 독립된 트랜잭션으로 실행되고 있습니다.
만약 saveAll() 실행 중 DB 예외가 발생하면, 채팅방은 이미 COMPLETED로 변경되었지만 시스템 메시지는 영원히 저장되지 않는 데이터 정합성 깨짐이 발생합니다.
스케쥴러는 단순히 흐름만 제어하고, 실제 비즈니스 로직(상태 변경 + 메시지 저장)은 별도의 Service 메서드로 추출하여 @Transactional로 묶어주어야 원자성(Atomicity)을 보장할 수 있습니다.

2. Bulk Update와 동시성 제어의 모순
주석에 "분산락이 1대만 허용하므로 비관적 락 없이 Bulk Update 사용" 이라고 작성하셨습니다. 이는 치명적인 오류입니다.
분산락(Lettuce)은 오직 스케쥴러 간의 중복 실행만을 막아줄 뿐입니다. 스케쥴러가 findInactiveRooms로 데이터를 조회한 후 bulkCompleteRooms()를 실행하기 전 찰나의 순간에, 일반 사용자 요청이 동일한 채팅방을 수정하려고 할 경우 동시성 제어가 전혀 작동하지 않습니다. (No-Offset 조회 시점에는 락이 없으므로)
비관적 락을 생략하려면, Bulk Update 쿼리의 WHERE 절에 chatRoom.chatRoomStatus.in(WAITING, IN_PROGRESS) 조건을 명시하여, 변경 중 상태가 바뀌었다면 업데이트 대상에서 제외되도록 방어해야 합니다.

💡 학습 포인트

  • JPQL Bulk Update와 영속성 컨텍스트: JPA의 execute()는 영속성 컨텍스트를 우회하여 DB에 직접 쿼리를 낸다는 점을 정확히 인지하고 em.clear()로 대응한 점이 훌륭합니다. 이는 JPA를 다루는 엔지니어의 기본 소양입니다.

🤔 생각해보기

현재 구조는 비활성 채팅방을 배치 사이즈(100개) 단위로 묶어 처리합니다. 만약 이 배치 단위의 처리(조회 -> 상태 변경 -> 메시지 저장)를 하나의 트랜잭션으로 묶는다면, 10만 개의 방을 처리할 때 DB에는 어떤 형태의 락(Lock)이 어떻게 걸리게 될까요? 이 구조가 대규모 트래픽 환경에서 다른 비즈니스 로직에 어떤 연쇄적인 장애를 유발할 수 있을지 고민해 보시기 바랍니다.

💬 이 질문에 대해 궁금한 점이 있으면 코멘트에 @sparta 를 남겨보세요!
예: @sparta 대규모 배치 처리 시 트랜잭션 범위를 쪼개는 기준이 무엇인가요?


AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.

@CheatIsKey CheatIsKey merged commit 2e172bc into develop Apr 24, 2026
2 checks passed
@CheatIsKey CheatIsKey deleted the feat/Chat-Inactivity-Scheduler#113 branch April 24, 2026 17:36
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Allday-Project-Board Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Feat] 실시간 채팅 자동 종료 스케쥴러

3 participants