Hotfix : 스케줄러 트랜잭션 분리로 알림 처리 중 JPA flush 오류 방지 #310
Conversation
📝 WalkthroughWalkthrough알람 리마인더 스케줄러에 새로운 공개 메서드 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit bd6a861. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/umc/teumteum/server/global/scheduler/RemindAlarmScheduler.java (3)
26-48: 트랜잭션 분리가 실제로 이루어지지 않음 (PR 목적 미달성)PR 설명에 따르면 DB 선점 로직과 알림 발송 로직을 별도 트랜잭션으로 분리하여 JPA flush 오류를 방지하는 것이 목적입니다. 하지만 현재 구조에서는:
remindAlarm()이@Transactional로 트랜잭션을 시작processNotifications()를 동일 메서드 내에서 직접 호출- Spring의 기본 트랜잭션 전파(REQUIRED)에 따라
processNotifications()는remindAlarm()의 트랜잭션에 참여결과적으로 여전히 모든 로직(DB 선점 + 알림 발송 + 실패 처리)이 단일 트랜잭션에서 실행되어, 알림 발송 중 예외 발생 시 Hibernate Session 오염 문제가 여전히 발생할 수 있습니다.
🔎 트랜잭션 분리를 위한 해결 방안
방안 1: Self-injection 패턴 사용 (권장)
@Slf4j @Component @RequiredArgsConstructor public class RemindAlarmScheduler { private final ScheduleReminderRepository scheduleReminderRepository; private final NotificationUseCases notificationUseCases; + private final RemindAlarmScheduler self; // Self-injection @Transactional @Scheduled(fixedRate = 60000, zone = "Asia/Seoul") public void remindAlarm() { LocalDateTime now = LocalDateTime.now(); List<ScheduleReminder> targets = scheduleReminderRepository.findDueWithScheduleAndUser( AlarmStatus.ACTIVE, DispatchStatus.PENDING, now, PageRequest.of(0, 100) ); if (targets.isEmpty()) return; List<Long> targetIds = targets.stream().map(ScheduleReminder::getId).toList(); int lockedCount = scheduleReminderRepository.updateDispatchStatus(targetIds, DispatchStatus.PENDING, DispatchStatus.PROCESSING); if (lockedCount == 0) return; - processNotifications(targetIds); + self.processNotifications(targetIds); // 프록시를 통한 호출로 새 트랜잭션 시작 } + @Transactional(propagation = Propagation.REQUIRES_NEW) public void processNotifications(List<Long> targetIds) { // ... 기존 로직 } }방안 2: 별도 서비스로 분리
알림 발송 로직을 별도의
@Service컴포넌트로 분리하여 트랜잭션 경계를 명확히 구분합니다.@Service @RequiredArgsConstructor public class NotificationProcessor { private final ScheduleReminderRepository scheduleReminderRepository; private final NotificationUseCases notificationUseCases; @Transactional(propagation = Propagation.REQUIRES_NEW) public void processNotifications(List<Long> targetIds) { // 기존 processNotifications 로직 } }그리고
RemindAlarmScheduler에서 주입받아 사용:@Component @RequiredArgsConstructor public class RemindAlarmScheduler { private final NotificationProcessor notificationProcessor; @Transactional @Scheduled(fixedRate = 60000, zone = "Asia/Seoul") public void remindAlarm() { // ... 락 획득 로직 notificationProcessor.processNotifications(targetIds); } }Based on coding guidelines: 트랜잭션 관리 - @transactional이 필요한 메소드에 누락되지 않았는지, DB 일관성, 롤백 정책이 올바른지 검토
49-66: 트랜잭션 관리 누락
processNotifications()메서드가 DB 조회(line 52)와 업데이트(lines 62-64)를 수행하지만@Transactional어노테이션이 없습니다.현재는
remindAlarm()의 트랜잭션에 참여하므로 문제가 없어 보이지만, 트랜잭션 분리가 제대로 구현되면 이 메서드는 독립적인 트랜잭션 관리가 필요합니다. 특히 실패 시 상태를 FAILED로 업데이트하는 로직(lines 62-64)은 원자적으로 처리되어야 합니다.🔎 제안 수정
+ @Transactional(propagation = Propagation.REQUIRES_NEW) public void processNotifications(List<Long> targetIds) { // ... 기존 로직 }Based on coding guidelines: 트랜잭션 관리 - @transactional이 필요한 메소드에 누락되지 않았는지 확인
56-65: 예외 처리 개선 권장현재 예외 처리 로직은 기본적으로 적절하나, 다음 개선사항을 고려해 주세요:
상태 업데이트 실패 처리: Lines 62-64의
updateDispatchStatusByIds()호출 자체가 실패할 경우 처리가 없습니다. 이 경우 리마인더가 PROCESSING 상태로 남아 재처리되지 않을 수 있습니다.모니터링 강화: 실패한 알림 건수를 메트릭으로 수집하면 운영 중 문제를 조기에 발견할 수 있습니다.
🔎 개선 제안
try{ notificationUseCases.notifyReminder(locked); } catch(Exception e){ log.error("리마인드 알림 발송 실패. locked count={}", locked.size(), e); - // PROCESSING 상태인 리마인더들 -> FAILED로 변경 - List<Long> lockedIds = locked.stream().map(ScheduleReminder::getId).toList(); - scheduleReminderRepository.updateDispatchStatusByIds( - lockedIds, DispatchStatus.PROCESSING, DispatchStatus.FAILED - ); + try { + // PROCESSING 상태인 리마인더들 -> FAILED로 변경 + List<Long> lockedIds = locked.stream().map(ScheduleReminder::getId).toList(); + int failedCount = scheduleReminderRepository.updateDispatchStatusByIds( + lockedIds, DispatchStatus.PROCESSING, DispatchStatus.FAILED + ); + log.info("실패 상태 업데이트 완료. count={}", failedCount); + } catch (Exception updateException) { + log.error("실패 상태 업데이트 중 오류 발생. PROCESSING 상태로 남은 리마인더가 있을 수 있음", updateException); + // 모니터링 알림 또는 메트릭 수집 고려 + } }Based on coding guidelines: 예외 처리 - 예외가 적절히 처리되었는지 확인, 공통 예외 처리 모듈 활용 여부 검토
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/umc/teumteum/server/global/scheduler/RemindAlarmScheduler.java
🧰 Additional context used
📓 Path-based instructions (1)
src/**
⚙️ CodeRabbit configuration file
src/**: 다음 항목들을 꼼꼼하게 검토해줘.
- 예외 처리
- 예외가 적절히 처리되었는지 확인해줘. (try-catch, throws, ExceptionAdvice)
- 공통 예외 처리 모듈(예: GlobalHandler, ApiResponse 등)을 잘 활용했는지 확인.
- RuntimeException을 남발하지 않고, 의미 있는 커스텀 예외를 사용하는지 검토.
- 예외 메시지에 민감 정보(DB 정보, 사용자 정보 등)가 노출되지 않게 했는지 점검.
- 코드 품질 & 가독성
- 메소드/클래스가 단일 책임 원칙(SRP)에 맞게 구성되어 있는지.
- 중복 코드가 있는 경우, 유틸/공통 컴포넌트로 추출 가능한지.
- 의미 있는 변수명과 메소드명을 사용했는지.
- 매직 넘버, 하드코딩된 값이 존재하는지 점검.
- 성능 및 효율성
- 불필요한 DB 쿼리 호출, N+1 문제 가능성이 있는지 확인.
- Stream, loop, recursion 사용 시 시간복잡도/메모리 효율성을 고려했는지.
- 캐시 적용 가능성이 있거나, 과도한 연산이 반복되는 구간이 있는지.
- 트랜잭션 관리
- @transactional이 필요한 메소드에 누락되지 않았는지.
- 읽기 전용 트랜잭션(readOnly = true)을 적절히 사용했는지.
- DB 일관성, 롤백 정책이 올바른지 검토.
- 입력 검증 및 보안
- @Valid, Bean Validation 등을 통한 입력값 검증이 되어 있는지.
- 비밀번호, 토큰 등 민감한 정보가 로깅되지 않는지.
- 테스트
- 단위 테스트가 충분히 작성되었는지, 핵심 로직의 검증이 누락되지 않았는지.
- Mocking을 통한 독립 테스트 구조를 유지했는지.
- 경계값 테스트, 예외 케이스 테스트가 포함되어 있는지.
- 구조 및 설계
- Controller, Service, Repository 등 계층 구조가 올바르게 나뉘어 있는지.
- DTO, Entity, Domain 객체 간 변환 로직이 명확하고 중복되지 않는지.
- Config 클래스에서 Bean 등록이 과도하거나 순환 참조 위험이 없는지.
Files:
src/main/java/umc/teumteum/server/global/scheduler/RemindAlarmScheduler.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Continuous Integration
| processNotifications(targetIds); | ||
|
|
||
| } | ||
| public void processNotifications(List<Long> targetIds) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
메서드 가시성 검토 필요
processNotifications() 메서드가 public으로 선언되어 있습니다.
- 트랜잭션 분리를 위해 self-injection 패턴을 사용할 경우
public이어야 하므로 현재 선언은 적절합니다. - 하지만 외부에서 이 메서드를 직접 호출하는 것은 의도되지 않은 동작을 유발할 수 있습니다.
- 만약 별도 서비스로 분리하는 방안을 선택한다면, 이 메서드는 외부 노출을 제한하는 것이 좋습니다.
권장사항:
- Self-injection 패턴 사용 시: 현재처럼
public유지하되, JavaDoc으로 "스케줄러 내부 전용" 명시 - 별도 서비스 분리 시: 외부 컴포넌트에서 호출 가능하도록 설계하거나, 접근 제한 고려
Based on coding guidelines: 구조 및 설계 - 계층 구조가 올바르게 나뉘어 있는지 확인
🤖 Prompt for AI Agents
In src/main/java/umc/teumteum/server/global/scheduler/RemindAlarmScheduler.java
around line 49, the processNotifications(List<Long> targetIds) method is
declared public but is intended only for scheduler/self-invocation; if you keep
the self-injection / transaction-proxy approach, leave it public and add a
JavaDoc comment above the method clearly stating "scheduler-internal only — do
not call from other components" (and optionally annotate internal use),
otherwise if you refactor this logic into a separate service change the method
visibility to package-private (or protected) and expose a controlled public API
on the new service instead so external callers cannot directly invoke the
scheduler internals.
👀 관련 이슈
기존 @scheduled 알림 스케줄러에서 DB 상태 변경과 외부 알림 발송 로직이 하나의 트랜잭션 안에서 실행되고 있었습니다.
이 구조에서 알림 발송 중 예외가 발생할 경우, Hibernate Session이 오염된 상태로 추가적인 DB flush가 시도되어 아래와 같은 런타임 오류가 발생하는 것을 확인했습니다.
✨ 작업한 내용
🌀 PR Point
x
🍰 참고사항
비즈니스 로직 변경은 없습니다!
📷 스크린샷 또는 GIF
Summary by CodeRabbit
릴리즈 노트
✏️ Tip: You can customize this high-level summary in your review settings.