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] 자동 예약 및 자동 업데이트 동시성 제어(#586) #604

Merged
merged 9 commits into from
Oct 16, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

import corea.scheduler.domain.AutomaticMatching;
import corea.scheduler.domain.ScheduleStatus;
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;

import java.util.List;
import java.util.Optional;

public interface AutomaticMatchingRepository extends JpaRepository<AutomaticMatching, Long> {

Optional<AutomaticMatching> findByRoomId(long roomId);
@Lock(LockModeType.PESSIMISTIC_WRITE)
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 의미하는 낙관적 락이
실제 DB 에 락을 걸어서 다른 서버의 트랜잭션에서 접근하지 못하게 막는다는 뜻 맞나요?

Copy link
Contributor

Choose a reason for hiding this comment

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

비관적 락이고 PERSSIMISTIC_WRITE 이기 때문에 베타적 잠금으로 동작할 것 같습니다.
그렇다고 하면 락을 획득한 쓰레드 외엔 락올 놓기 전까지 update, delete, insert 가 막히겠군요!

Copy link
Contributor

Choose a reason for hiding this comment

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

실제 DB 에 락을 걸어서 다른 서버의 트랜잭션에서 접근하지 못하게 막는다는 뜻 맞나요?

넵 맞습니다. @Lock을 사용하면 실제 DB단에 락을 걸어 다른 서버 간 동시성도 막을 수 있습니다.

@Query("SELECT am FROM AutomaticMatching am WHERE am.roomId = :roomId AND am.status = :status")
Optional<AutomaticMatching> findByRoomIdAndStatusForUpdate(long roomId, ScheduleStatus status);

List<AutomaticMatching> findAllByStatus(ScheduleStatus status);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

import corea.scheduler.domain.AutomaticUpdate;
import corea.scheduler.domain.ScheduleStatus;
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;

import java.util.List;
import java.util.Optional;

public interface AutomaticUpdateRepository extends JpaRepository<AutomaticUpdate, Long> {

Optional<AutomaticUpdate> findByRoomId(long roomId);
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("SELECT au FROM AutomaticUpdate au WHERE au.roomId = :roomId AND au.status = :status")
Optional<AutomaticUpdate> findByRoomIdAndStatusForUpdate(long roomId, ScheduleStatus status);

List<AutomaticUpdate> findAllByStatus(ScheduleStatus status);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,88 +1,24 @@
package corea.scheduler.service;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.matching.domain.PullRequestInfo;
import corea.matching.service.MatchingService;
import corea.matching.service.PullRequestProvider;
import corea.matchresult.domain.FailedMatching;
import corea.matchresult.repository.FailedMatchingRepository;
import corea.room.domain.Room;
import corea.room.repository.RoomRepository;
import corea.scheduler.domain.AutomaticMatching;
import corea.scheduler.domain.ScheduleStatus;
import corea.scheduler.repository.AutomaticMatchingRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.support.TransactionTemplate;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Component
@RequiredArgsConstructor
public class AutomaticMatchingExecutor {

private final PlatformTransactionManager transactionManager;
private final MatchingService matchingService;
private final PullRequestProvider pullRequestProvider;
private final RoomRepository roomRepository;
private final FailedMatchingRepository failedMatchingRepository;
private final MatchingExecutor matchingExecutor;
private final AutomaticMatchingRepository automaticMatchingRepository;

@Async
@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

의도에 맞게 잘 변경된거 같아요 👍

public void execute(long roomId) {
//TODO: 트랜잭션 분리
TransactionTemplate template = new TransactionTemplate(transactionManager);

try {
template.execute(status -> {
startMatching(roomId);
return null;
});
} catch (CoreaException e) {
log.warn("매칭 실행 중 에러 발생: {}", e.getMessage(), e);
recordMatchingFailure(roomId, e.getExceptionType());
}
}

private void startMatching(long roomId) {
Room room = getRoom(roomId);

PullRequestInfo pullRequestInfo = pullRequestProvider.getUntilDeadline(room.getRepositoryLink(), room.getRecruitmentDeadline());
matchingService.match(roomId, pullRequestInfo);

AutomaticMatching automaticMatching = getAutomaticMatchingByRoomId(roomId);
automaticMatching.updateStatusToDone();
}

private void recordMatchingFailure(long roomId, ExceptionType exceptionType) {
//TODO: 위와 동일
TransactionTemplate template = new TransactionTemplate(transactionManager);
template.execute(status -> {
updateRoomStatusToFail(roomId);
saveFailedMatching(roomId, exceptionType);
return null;
});
}

private void updateRoomStatusToFail(long roomId) {
Room room = getRoom(roomId);
room.updateStatusToFail();
}

private void saveFailedMatching(long roomId, ExceptionType exceptionType) {
FailedMatching failedMatching = new FailedMatching(roomId, exceptionType);
failedMatchingRepository.save(failedMatching);
}

private Room getRoom(long roomId) {
return roomRepository.findById(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.ROOM_NOT_FOUND));
}

private AutomaticMatching getAutomaticMatchingByRoomId(long roomId) {
return automaticMatchingRepository.findByRoomId(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.AUTOMATIC_MATCHING_NOT_FOUND));
automaticMatchingRepository.findByRoomIdAndStatusForUpdate(roomId, ScheduleStatus.PENDING)
.ifPresent(automaticMatching -> {
matchingExecutor.match(roomId);
automaticMatching.updateStatusToDone();
});
}
}
Original file line number Diff line number Diff line change
@@ -1,88 +1,24 @@
package corea.scheduler.service;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.feedback.domain.DevelopFeedback;
import corea.feedback.domain.SocialFeedback;
import corea.feedback.repository.DevelopFeedbackRepository;
import corea.feedback.repository.SocialFeedbackRepository;
import corea.matchresult.domain.MatchResult;
import corea.matchresult.domain.ReviewStatus;
import corea.matchresult.repository.MatchResultRepository;
import corea.member.domain.Member;
import corea.member.domain.MemberRole;
import corea.room.domain.Room;
import corea.room.repository.RoomRepository;
import corea.scheduler.domain.AutomaticUpdate;
import corea.scheduler.domain.ScheduleStatus;
import corea.scheduler.repository.AutomaticUpdateRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Component
@RequiredArgsConstructor
public class AutomaticUpdateExecutor {

private final RoomRepository roomRepository;
private final MatchResultRepository matchResultRepository;
private final SocialFeedbackRepository socialFeedbackRepository;
private final DevelopFeedbackRepository developFeedbackRepository;
private final UpdateExecutor updateExecutor;
private final AutomaticUpdateRepository automaticUpdateRepository;

@Async
@Transactional
public void execute(long roomId) {
Room room = getRoom(roomId);
room.updateStatusToClose();

updateReviewCount(roomId);
updateFeedbackPoint(roomId);

AutomaticUpdate automaticUpdate = getAutomaticUpdateByRoomId(roomId);
automaticUpdate.updateStatusToDone();
}

private void updateReviewCount(long roomId) {
matchResultRepository.findAllByRoomIdAndReviewStatus(roomId, ReviewStatus.COMPLETE)
.forEach(this::increaseMembersReviewCountIn);
}

private void increaseMembersReviewCountIn(MatchResult matchResult) {
Member reviewer = matchResult.getReviewer();
reviewer.increaseReviewCount(MemberRole.REVIEWER);

Member reviewee = matchResult.getReviewee();
reviewee.increaseReviewCount(MemberRole.REVIEWEE);
}

private void updateFeedbackPoint(long roomId) {
socialFeedbackRepository.findAllByRoomId(roomId)
.forEach(this::updateSocialFeedbackPoint);

developFeedbackRepository.findAllByRoomId(roomId)
.forEach(this::updateDevelopFeedbackPoint);
}

private void updateSocialFeedbackPoint(SocialFeedback socialFeedback) {
Member receiver = socialFeedback.getReceiver();
receiver.updateAverageRating(socialFeedback.getEvaluatePoint());
}

private void updateDevelopFeedbackPoint(DevelopFeedback developFeedback) {
Member receiver = developFeedback.getReceiver();
receiver.updateAverageRating(developFeedback.getEvaluatePoint());
}

private Room getRoom(long roomId) {
return roomRepository.findById(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.ROOM_NOT_FOUND));
}

private AutomaticUpdate getAutomaticUpdateByRoomId(long roomId) {
return automaticUpdateRepository.findByRoomId(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.AUTOMATIC_UPDATE_NOT_FOUND));
automaticUpdateRepository.findByRoomIdAndStatusForUpdate(roomId, ScheduleStatus.PENDING)
.ifPresent(automaticUpdate -> {
updateExecutor.update(roomId);
automaticUpdate.updateStatusToDone();
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package corea.scheduler.service;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.matching.domain.PullRequestInfo;
import corea.matching.service.MatchingService;
import corea.matching.service.PullRequestProvider;
import corea.room.domain.Room;
import corea.room.repository.RoomRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;

@Slf4j
@Component
@RequiredArgsConstructor
public class MatchingExecutor {

private final PlatformTransactionManager transactionManager;
private final PullRequestProvider pullRequestProvider;
private final MatchingService matchingService;
private final RoomRepository roomRepository;

@Async
public void match(long roomId) {
//TODO: 트랜잭션 분리
Copy link
Contributor

Choose a reason for hiding this comment

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

트랜잭션 분리라는 게 정확히 어떤 태스크를 뜻하는 건가요? ㅇ.ㅇ
이번 PR에서 수행된 TODO인가요?

Copy link
Contributor

Choose a reason for hiding this comment

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

제가 적어둔 "트랜잭션 분리" TODO는 매칭 실패 시 예외가 발생했을 때 데이터를 저장하기 위해 명시적으로 트랜잭션을 선언한 부분입니다.

현재는 코드에서 직접 TransactionTemplate을 사용해 트랜잭션을 관리하고 있지만, 이를 더 효율적으로 관리하기 위해 트랜잭션 전파 속성을 활용해 별도의 클래스로 분리할 계획입니다.

TransactionTemplate template = new TransactionTemplate(transactionManager);

try {
template.execute(status -> {
startMatching(roomId);
return null;
});
} catch (CoreaException e) {
log.warn("매칭 실행 중 에러 발생: {}", e.getMessage(), e);
updateRoomStatusToFail(roomId);
}
}

private void startMatching(long roomId) {
Room room = getRoom(roomId);
PullRequestInfo pullRequestInfo = pullRequestProvider.getUntilDeadline(room.getRepositoryLink(), room.getRecruitmentDeadline());

matchingService.match(roomId, pullRequestInfo);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change


private void updateRoomStatusToFail(long roomId) {
//TODO: 위와 동일
TransactionTemplate template = new TransactionTemplate(transactionManager);
template.execute(status -> {
Room room = getRoom(roomId);
room.updateStatusToFail();
return null;
});
}

private Room getRoom(long roomId) {
return roomRepository.findById(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.ROOM_NOT_FOUND));
}
}
75 changes: 75 additions & 0 deletions backend/src/main/java/corea/scheduler/service/UpdateExecutor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package corea.scheduler.service;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.feedback.domain.DevelopFeedback;
import corea.feedback.domain.SocialFeedback;
import corea.feedback.repository.DevelopFeedbackRepository;
import corea.feedback.repository.SocialFeedbackRepository;
import corea.matchresult.domain.MatchResult;
import corea.matchresult.domain.ReviewStatus;
import corea.matchresult.repository.MatchResultRepository;
import corea.member.domain.Member;
import corea.member.domain.MemberRole;
import corea.room.domain.Room;
import corea.room.repository.RoomRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

@Component
@RequiredArgsConstructor
public class UpdateExecutor {

private final RoomRepository roomRepository;
private final MatchResultRepository matchResultRepository;
private final SocialFeedbackRepository socialFeedbackRepository;
private final DevelopFeedbackRepository developFeedbackRepository;

@Async
@Transactional
public void update(long roomId) {
Room room = getRoom(roomId);
room.updateStatusToClose();

updateReviewCount(roomId);
updateFeedbackPoint(roomId);
}

private void updateReviewCount(long roomId) {
matchResultRepository.findAllByRoomIdAndReviewStatus(roomId, ReviewStatus.COMPLETE)
.forEach(this::increaseMembersReviewCountIn);
}

private void increaseMembersReviewCountIn(MatchResult matchResult) {
Member reviewer = matchResult.getReviewer();
reviewer.increaseReviewCount(MemberRole.REVIEWER);

Member reviewee = matchResult.getReviewee();
reviewee.increaseReviewCount(MemberRole.REVIEWEE);
}

private void updateFeedbackPoint(long roomId) {
socialFeedbackRepository.findAllByRoomId(roomId)
.forEach(this::updateSocialFeedbackPoint);

developFeedbackRepository.findAllByRoomId(roomId)
.forEach(this::updateDevelopFeedbackPoint);
}

private void updateSocialFeedbackPoint(SocialFeedback socialFeedback) {
Member receiver = socialFeedback.getReceiver();
receiver.updateAverageRating(socialFeedback.getEvaluatePoint());
}

private void updateDevelopFeedbackPoint(DevelopFeedback developFeedback) {
Member receiver = developFeedback.getReceiver();
receiver.updateAverageRating(developFeedback.getEvaluatePoint());
}

private Room getRoom(long roomId) {
return roomRepository.findById(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.ROOM_NOT_FOUND));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import org.junit.jupiter.params.provider.CsvSource;
Copy link
Contributor

Choose a reason for hiding this comment

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

submodule 쪽에 의견을 남길수 없어서 여기 남깁니다.
config push 한 내용을 제거할 수 있으면 제거해주세요
( 최신 버전에서 10월 4일 cfed68 로 변경된거 같아요 )

Copy link
Contributor

Choose a reason for hiding this comment

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

그러게요 예전 커밋으로 돌아갔네요
확인 부탁드려요!!

Copy link
Contributor

Choose a reason for hiding this comment

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

확인이요~! 수정합니당~


import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

class MatchingFailedReasonTest {

Expand Down
Loading
Loading