refactor: handle 트랜잭션 제거 및 SessionPersistenceService 분리#70
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the session completion and failure logic by introducing a new SessionPersistenceService and removing the @Transactional annotation from the asynchronous AllAnalysisCompletedHandler.handle method. The review identifies a critical concurrency risk where multiple threads could simultaneously pass state checks and trigger redundant, expensive feedback generation. Furthermore, the feedback suggests improving data integrity by passing session IDs to the persistence service and reloading entities within the transaction to prevent overwriting changes and to utilize JPA's dirty checking mechanism.
| @Async("videoTaskExecutor") | ||
| @EventListener | ||
| @Transactional | ||
| public void handle(AllAnalysisCompletedEvent event) { |
There was a problem hiding this comment.
handle 메서드에서 @transactional이 제거되고 @async로 동작함에 따라, 여러 답변이 동시에 완료될 경우 isFinalFeedbackReady 체크를 여러 스레드가 동시에 통과할 가능성이 있습니다. 이로 인해 비용이 많이 드는 feedbackGenerator.generateFinal() 호출이 중복으로 발생하고, 최종 결과가 여러 번 저장되거나 SSE 이벤트가 중복 전송될 수 있습니다. 세션의 상태를 '분석 중' 등의 중간 상태로 변경하여 원자적으로 확인하거나, 분산 락 등을 고려하여 중복 실행을 방지하는 것이 좋습니다.
| package io.wisoft.prepair.prepair_api.interview.session.service; | ||
|
|
||
| import io.wisoft.prepair.prepair_api.interview.session.entity.InterviewSession; | ||
| import io.wisoft.prepair.prepair_api.interview.session.repository.SessionRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class SessionPersistenceService { | ||
|
|
||
| private final SessionRepository sessionRepository; | ||
|
|
||
| @Transactional | ||
| public void saveCompletedSession(InterviewSession session, int finalScore, String finalFeedback) { | ||
| session.complete(finalScore, finalFeedback); | ||
| sessionRepository.save(session); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void saveFailedSession(InterviewSession session) { | ||
| session.fail(); | ||
| sessionRepository.save(session); | ||
| } | ||
| } |
There was a problem hiding this comment.
준영속 상태의 엔티티를 save()(merge)하는 방식은 AI 분석과 같은 긴 작업 도중 다른 트랜잭션에 의해 변경된 데이터를 덮어쓸 위험이 있습니다. UUID를 전달받아 트랜잭션 내에서 엔티티를 다시 조회하고 상태를 변경하는 것이 더 안전합니다. 또한, 영속성 컨텍스트의 변경 감지(Dirty Checking) 기능을 활용하면 명시적인 save() 호출 없이도 변경 사항을 반영할 수 있습니다.
package io.wisoft.prepair.prepair_api.interview.session.service;
import io.wisoft.prepair.prepair_api.interview.session.entity.InterviewSession;
import io.wisoft.prepair.prepair_api.interview.session.entity.SessionStatus;
import io.wisoft.prepair.prepair_api.interview.session.repository.SessionRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.util.UUID;
@Service
@RequiredArgsConstructor
public class SessionPersistenceService {
private final SessionRepository sessionRepository;
@Transactional
public void saveCompletedSession(UUID sessionId, int finalScore, String finalFeedback) {
InterviewSession session = sessionRepository.findById(sessionId)
.orElseThrow(() -> new IllegalArgumentException("존재하지 않는 세션입니다."));
if (session.getStatus() == SessionStatus.COMPLETED) {
return;
}
session.complete(finalScore, finalFeedback);
}
@Transactional
public void saveFailedSession(UUID sessionId) {
InterviewSession session = sessionRepository.findById(sessionId)
.orElseThrow(() -> new IllegalArgumentException("존재하지 않는 세션입니다."));
if (session.getStatus() != SessionStatus.IN_PROGRESS) {
return;
}
session.fail();
}
}|
|
||
| session.complete(data.finalScore(), finalResult.finalFeedback()); | ||
| sessionRepository.save(session); | ||
| sessionPersistenceService.saveCompletedSession(session, data.finalScore(), finalResult.finalFeedback()); |
There was a problem hiding this comment.
| InterviewSession session = answer.getInterviewQuestion().getInterviewSession(); | ||
| session.fail(); | ||
| sessionRepository.save(session); | ||
| sessionPersistenceService.saveFailedSession(session); |
📝 작업 내용 (Description)
handle 트랜잭션 제거 및 SessionPersistenceService 분리
🔄 변경 유형 (Type of Change)
✅ 체크리스트 (Checklist)