Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
public class UserCommandPersistenceAdapter implements UserCommandPort {

private final UserJpaRepository userJpaRepository;

private final UserMapper userMapper;

@Override
Expand All @@ -38,6 +37,14 @@ public User findById(Long userId) {
return userMapper.toDomainEntity(userJpaEntity);
}

@Override
public User findByIdWithLock(Long userId) {
UserJpaEntity userJpaEntity = userJpaRepository.findByUserIdWithLock(userId).orElseThrow(
() -> new EntityNotFoundException(USER_NOT_FOUND));

return userMapper.toDomainEntity(userJpaEntity);
}

@Override
public Map<Long, User> findByIds(List<Long> userIds) {
List<UserJpaEntity> entities = userJpaRepository.findAllById(userIds); // 내부 구현 메서드가 jpql 기반이므로 필터 적용 대상임을 확인함
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package konkuk.thip.user.adapter.out.persistence.repository;

import jakarta.persistence.LockModeType;
import jakarta.persistence.QueryHint;
import konkuk.thip.user.adapter.out.jpa.UserJpaEntity;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.jpa.repository.QueryHints;
import org.springframework.data.repository.query.Param;

import java.util.List;
Expand All @@ -15,6 +19,13 @@ public interface UserJpaRepository extends JpaRepository<UserJpaEntity, Long>, U
*/
Optional<UserJpaEntity> findByUserId(Long userId);

@Lock(LockModeType.PESSIMISTIC_WRITE)
@QueryHints({
@QueryHint(name = "jakarta.persistence.lock.timeout", value = "5000") // 5초
})
@Query("select u from UserJpaEntity u where u.userId = :userId")
Optional<UserJpaEntity> findByUserIdWithLock(@Param("userId") Long userId);

Optional<UserJpaEntity> findByOauth2Id(String oauth2Id);

boolean existsByNickname(String nickname);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public interface UserCommandPort {

Long save(User user);
User findById(Long userId);
User findByIdWithLock(Long userId);
Map<Long, User> findByIds(List<Long> userIds);
void update(User user);
void delete(User user);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package konkuk.thip.user.application.service.following;

import konkuk.thip.common.exception.BusinessException;
import konkuk.thip.common.exception.InvalidStateException;
import konkuk.thip.common.exception.code.ErrorCode;
import konkuk.thip.notification.application.port.in.FeedNotificationOrchestrator;
import konkuk.thip.user.application.port.in.UserFollowUsecase;
import konkuk.thip.user.application.port.in.dto.UserFollowCommand;
Expand All @@ -9,12 +11,15 @@
import konkuk.thip.user.domain.Following;
import konkuk.thip.user.domain.User;
import lombok.RequiredArgsConstructor;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Recover;
import org.springframework.retry.annotation.Retryable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Optional;

import static konkuk.thip.common.exception.code.ErrorCode.*;
import static konkuk.thip.common.exception.code.ErrorCode.USER_CANNOT_FOLLOW_SELF;

@Service
@RequiredArgsConstructor
Expand All @@ -27,6 +32,18 @@ public class UserFollowService implements UserFollowUsecase {

@Override
@Transactional
@Retryable(
Copy link
Member

Choose a reason for hiding this comment

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

확인했습니다 ~~ 룻구투미~

notRecoverable = {
BusinessException.class,
InvalidStateException.class
},
noRetryFor = {
BusinessException.class,
InvalidStateException.class
},
Comment on lines +36 to +43
Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM 재시도 X, recover X 이렇게 설정하셨군요 좋습니다!

그런데 혹시 발생할 수 있는 재시도 상황에서 changeFollowingState() 메서드가 새로 트랜잭션을 시작하는게 아니라, 기존 트랜잭션을 이어가도록 하신 이유가 있을까요??

(락 타임아웃 상황이 비교적 긴 상황에서 재시도가 발생하는 상황이 있을까 싶긴하지만) 혹시나 발생할 수 있는 재시도 상황에서 기존 트랜잭션으로 서비스 메서드를 재시도하게 되면 해당 요청 쓰레드가 DB 커넥션을 그만큼 길게 물고있다고 생각해서 저는 매번 새로 DB 커넥션을 맺도록 트랜잭션 propagation 설정을 REQUIRES_NEW로 설정하긴 했습니다!

단순 궁금증에 여쭤봅니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사실 구현 당시에는 재시도 시 각 시도마다 독립적인 트랜잭션으로 수행되는 것으로 이해하고 별도로 설정을 추가하지 않았습니다. 다만 성준님 질문을 보고 궁금해서 Spring Retry 공식 내용을 확인해보니, 관련해서 논의된 이슈가 있었습니다.

해당 이슈(#22)에서는 AOP로 동작하는 @Retryable의 order 기본값을 Ordered.LOWEST_PRECEDENCE - 1로 두어, 일반적으로 LOWEST_PRECEDENCE인 @Transactional보다 Retry AOP가 먼저 적용되도록(바깥에서 감싸도록) 하는 방향을 다루고 있습니다.
즉 기본 구성에서는 @Retryable이 바깥, @Transactional이 안쪽 구조가 되어 호출 흐름이 @Retryable -> @Transactional 순서로 형성되고, 결과적으로 재시도 시에는 이전 시도의 트랜잭션이 종료(롤백)된 뒤 다음 시도에서 트랜잭션이 새로 시작되는 형태로 동작하게 됩니다.

따라서 현재 제 코드에서도 재시도는 매번 새로운 트랜잭션에서 수행되는 것으로 보시면 될 것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 새로운 정보를 알았네요! @EnableRetry 어노테이션 코드 확인하니 현준님이 말한 내용 명시되어 있는거 확인했습니다!
image

maxAttempts = 3,
backoff = @Backoff(delay = 100, maxDelay = 500, multiplier = 2)
)
public Boolean changeFollowingState(UserFollowCommand followCommand) {
Long userId = followCommand.userId();
Long targetUserId = followCommand.targetUserId();
Expand All @@ -35,7 +52,7 @@ public Boolean changeFollowingState(UserFollowCommand followCommand) {
validateParams(userId, targetUserId);

Optional<Following> optionalFollowing = followingCommandPort.findByUserIdAndTargetUserId(userId, targetUserId);
User targetUser = userCommandPort.findById(targetUserId);
User targetUser = userCommandPort.findByIdWithLock(targetUserId);

boolean isFollowRequest = Following.validateFollowingState(optionalFollowing.isPresent(), type);

Expand All @@ -53,6 +70,11 @@ public Boolean changeFollowingState(UserFollowCommand followCommand) {
}
}

@Recover
public Boolean recoverChangeFollowingState(Exception e, UserFollowCommand followCommand) {
throw new BusinessException(ErrorCode.RESOURCE_LOCKED);
}

private void sendNotifications(Long userId, Long targetUserId) {
User actorUser = userCommandPort.findById(userId);
feedNotificationOrchestrator.notifyFollowed(targetUserId, actorUser.getId(), actorUser.getNickname());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- 팔로잉 테이블에 사용자와 타겟 사용자 간의 유니크 제약 조건 추가
ALTER TABLE followings
ADD CONSTRAINT uq_followings_user_target
UNIQUE (user_id, following_user_id);
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void follow_newRelation() {
.thenReturn(Optional.empty());

User user = createUserWithFollowerCount(0);
when(userCommandPort.findById(targetUserId)).thenReturn(user);
when(userCommandPort.findByIdWithLock(targetUserId)).thenReturn(user);
when(userCommandPort.findById(userId)).thenReturn(user); // 알림 전송용

UserFollowCommand command = new UserFollowCommand(userId, targetUserId, true);
Expand Down Expand Up @@ -106,7 +106,7 @@ void unfollow_existingRelation() {

when(followingCommandPort.findByUserIdAndTargetUserId(userId, targetUserId))
.thenReturn(Optional.of(existing));
when(userCommandPort.findById(targetUserId)).thenReturn(user);
when(userCommandPort.findByIdWithLock(targetUserId)).thenReturn(user);

UserFollowCommand command = new UserFollowCommand(userId, targetUserId, false);

Expand Down