-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: handle 트랜잭션 제거 및 SessionPersistenceService 분리 #70
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,14 +13,13 @@ | |
| import io.wisoft.prepair.prepair_api.interview.question.entity.InterviewQuestion; | ||
| import io.wisoft.prepair.prepair_api.interview.question.repository.QuestionRepository; | ||
| import io.wisoft.prepair.prepair_api.interview.session.entity.InterviewSession; | ||
| import io.wisoft.prepair.prepair_api.interview.session.repository.SessionRepository; | ||
| import io.wisoft.prepair.prepair_api.interview.session.service.SessionPersistenceService; | ||
| import io.wisoft.prepair.prepair_api.common.support.SseEmitterManager; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.context.event.EventListener; | ||
| import org.springframework.scheduling.annotation.Async; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
|
|
@@ -42,12 +41,11 @@ public class AllAnalysisCompletedHandler { | |
| private final AnswerPersistenceService answerPersistenceService; | ||
| private final AnswerRepository answerRepository; | ||
| private final QuestionRepository questionRepository; | ||
| private final SessionRepository sessionRepository; | ||
| private final SessionPersistenceService sessionPersistenceService; | ||
| private final SseEmitterManager sseEmitterManager; | ||
|
|
||
| @Async("videoTaskExecutor") | ||
| @EventListener | ||
| @Transactional | ||
| public void handle(AllAnalysisCompletedEvent event) { | ||
| UUID answerId = event.answerId(); | ||
| deleteTempFile(event.videoPath()); | ||
|
|
@@ -201,8 +199,7 @@ private FinalFeedbackData buildFinalData( | |
| private void completeSession(InterviewSession session, FinalFeedbackData data, FinalFeedbackResult finalResult) { | ||
| UUID sessionId = session.getId(); | ||
|
|
||
| session.complete(data.finalScore(), finalResult.finalFeedback()); | ||
| sessionRepository.save(session); | ||
| sessionPersistenceService.saveCompletedSession(session, data.finalScore(), finalResult.finalFeedback()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| FinalFeedbackResponse response = new FinalFeedbackResponse( | ||
| sessionId, | ||
|
|
@@ -222,8 +219,7 @@ private void failSession(UUID answerId, String message) { | |
| if (answer == null || answer.getInterviewQuestion().getInterviewSession() == null) return; | ||
|
|
||
| InterviewSession session = answer.getInterviewQuestion().getInterviewSession(); | ||
| session.fail(); | ||
| sessionRepository.save(session); | ||
| sessionPersistenceService.saveFailedSession(session); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| sseEmitterManager.send(session.getId(), "analysis-failed", Map.of("message", message)); | ||
| sseEmitterManager.complete(session.getId()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| 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); | ||
| } | ||
| } | ||
|
Comment on lines
+1
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 준영속 상태의 엔티티를 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();
}
} |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle 메서드에서 @transactional이 제거되고 @async로 동작함에 따라, 여러 답변이 동시에 완료될 경우 isFinalFeedbackReady 체크를 여러 스레드가 동시에 통과할 가능성이 있습니다. 이로 인해 비용이 많이 드는 feedbackGenerator.generateFinal() 호출이 중복으로 발생하고, 최종 결과가 여러 번 저장되거나 SSE 이벤트가 중복 전송될 수 있습니다. 세션의 상태를 '분석 중' 등의 중간 상태로 변경하여 원자적으로 확인하거나, 분산 락 등을 고려하여 중복 실행을 방지하는 것이 좋습니다.