Skip to content

Conversation

kilhyeonjun
Copy link
Contributor

@kilhyeonjun kilhyeonjun commented Oct 31, 2024

요구사항

  • DB Lock 을 활용한 동시성 제어 방식 에서 해당 비즈니스 로직에서 적합하다고 판단하여 차용한 동시성 제어 방식을 구현하여 비즈니스 로직에 적용하고, 통합테스트 등으로 이를 검증하는 코드 작성 및 제출

작업 내용

redis 분산락 추가 (b10f7a5, 4e7849e)

#28 에서 좌석예약, 잔액 충전, 잔액 결제(예약 내역 결제)에 대해 분석하여 적용할 락을 정했습니다.

좌석 예약 분산락 추가 및 테스트 (7d6cc14)
사용자 잔액 충전 낙관적락, 분산락 추가 및 테스트 (9a1019e)
예약 내역 결제 낙관적락, 분산락 추가 및 테스트 (62d4876)

선택 동시성 제어 방법 API에 적용 (bc3da39)
좌석 예약 - 낙관적락
잔액 충전 - 분산락, 낙관적락
잔액 결제(예약 내역 결제) - 분산락, 낙관적락

코드 구현

각 usecase 별 비관적락, 낙관적락, 분산락 구현 및 통합 + 동시성 테스트를 진행했습니다.
사실 잔액과 좌석 entity에 @Version 어노테이션에 의해 모든 동시성제어에 낙관적락이 포함되어있습니다.
하지만 비관적락과 분산락의 경우 낙관적락에 의한 충돌이 안나오고 나온다면 error로 생각하고 작성했기에 감안해서 봐주시면 좋을 것 같습니다.

리뷰포인트

  • 리뷰어님들이 생각했을 때 더 좋은 패턴이 보일 경우 공유해주시면 좋을 것 같습니다.
  • 테스트 코드의 추가로 있으면 좋을 부분 또는 불 필요해 보이는 부분도 확인 부탁드립니다.

@kilhyeonjun kilhyeonjun self-assigned this Oct 31, 2024
@kilhyeonjun kilhyeonjun marked this pull request as ready for review October 31, 2024 11:37
Copy link

@jyjae jyjae left a comment

Choose a reason for hiding this comment

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

👍

@@ -110,6 +114,29 @@ public Reservation reserveConcertSeat(Long concertSeatId, Long userId) {
return concertQueryService.getReservation(new GetReservationByIdQuery(reservationId));
}

@Deprecated(forRemoval = false)
Copy link

Choose a reason for hiding this comment

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

@deprecated(forRemoval = false) 사용하신거 너무 좋은거 같습니다.


@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface DistributedLock {
Copy link

Choose a reason for hiding this comment

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

waitTime과 leaseTime은 안넣은 이유가 있으실까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

기본적으로 공통값을 가져가고 추후 변경이 필요하면 추가하는게 좋을 것 같다고 생각했어요!
수정되는 case가 비지니스가 느려져서 공통으로 설정된 값보다 커져야하는 경우일 것 같은데 이건 비지니스 속도를 올리는 방법으로 개선해야되지않을까해서 강제한 느낌도 있습니다!


@Aspect
@Component
@Order(Ordered.HIGHEST_PRECEDENCE + 1)
Copy link

Choose a reason for hiding this comment

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

order를 사용하신 이유가 있으실까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

트랜잭션 어노테이션과 같이 사용했을때 Lock 획득을 먼저하기위해서 사용했습니다. 트랜잭션 어노테이션의 순서는Ordered.HIGHEST_PRECEDENCE 이더라구여

@syjeon0608
Copy link

오늘도 많이 배워갑니당.. aop 활용은 안해봤는데 현준님 코드덕에 맛보고 갑니다 👍🏼

@kilhyeonjun kilhyeonjun merged commit 6aa1f0b into master Nov 3, 2024
@kilhyeonjun kilhyeonjun deleted the feature/step-12 branch November 3, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants