-
Notifications
You must be signed in to change notification settings - Fork 28
[3주차] 예약 API 구현 & 단계별 락 적용 #57
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
base: scars97
Are you sure you want to change the base?
Conversation
- 영화, 상영관, 상영일정 조회
- Mapper 추가 : Entity -> Domain Model
- 아키텍처, 멀티모듈 설계, ERD, 시퀀스 다이어그램 추가
- 기존 api 모듈 -> infra 모듈로 통합 - infra 모듈 : 외부 시스템 연동 역할 수행
- AvailableMovieResult: 상영 중인 영화 정보 Dto - MovieResult: 전체 영화 정보 Dto - Theater, Schedule Dto 생성
- 계층별 Dto 분리
- 제거한 이유에 대한 문서 작성
- QueryDsl 설정 - FullText Index 사용을 위한 MysqlDialect 커스텀
- k6 스크립트 추가
- Caffeine 제거(사용X)
- post 요청 시 엔티티 생성되지 않는 문제로 인한 수정
- 회원, 상영일정 검증 로직 추가 - 테스트 작성
- 상태 검증, 변경 로직 작성 - 테스트 작성 - 의존성 정리
- 좌석 개수, 연속 좌석 검증 - 예약 생성 - 테스트 작성
- app push 기능 이벤트 처리
- TestContainer 설정
- infra 모듈에서 제어하도록 수정
- 동시성 테스트 작성
- 동시성 테스트 리팩토링 (feat.CompletableFuture)
- TestContainer : Redis 추가
- AOP 분산락 적용
- 문장 정리
|
안녕하세요~ 리뷰 시작하겠습니다. |
|
|
|
||
|
|
||
| @RestController | ||
| @RequestMapping("/api/movies") |
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.
필요하시다면, api versioning 도 추가해도 좋을 듯 하네요 :) 실무에서는 버저닝을 /api/v1/movies 이런 형태로도 주로 하곤 합니다.
| data class CreateReservationRequest( | ||
| @field:NotNull(message = "회원 ID는 필수 입력 값입니다.") | ||
| @field:Positive(message = "회원 ID 는 0보다 큰 값이어야 합니다.") | ||
| val userId: Long, |
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.
각 필드들에 대한 validation 까지 잘 구현해주셨네요 :)
|
|
||
| fun validate(info: ReservationInfo) { | ||
| if (!userService.isUserExists(info.userId)) { | ||
| throw BusinessException(USER_NOT_FOUND, "존재하지 않는 회원 ID: ${info.userId}") |
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.
예외처리까지 꼼꼼하게 해주셨습니다 💯
| } | ||
| } | ||
|
|
||
| fun checkContinuousSeats(seats: List<Seat>) { |
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.
요거는 seat 에 두지 않고 reservation 에 두셨던 이유가 있을까요?
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.
Seat과 Reservation 중 어떤 곳에 둬야할 지, 아직까지도 고민 중인 부분입니다..
Reservation에 넣은 이유는
좌석 개수 제한, 연속된 좌석인지 검증하는 주체는 Reservation이라 생각했습니다.
좌석 데이터를 기준으로 검증하고 있지만 결국 예약이라는 문제를 해결하기 위한 수단이라 생각하여,
구현 당시에는 Reservation 안에 있는 게 맞지 않을까 생각했습니다..!
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.
넵 이유로서는 괜찮은 것 같습니다 :!!
| class MovieApplication | ||
|
|
||
| fun main(args: Array<String>) { | ||
| runApplication<MovieApplication>(*args) |
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.
특이하게 infrastructure 를 Application 으로 설정하셨네요 :) 저는 application 을 생각했었는데, 명확하게 presentation 계층으로 해도 명확하지 않을까? 생각해봤습니다.
| seatService.updateForReserve(info.seatIds, reservation.reservationId) | ||
|
|
||
| // App push 이벤트 발행 | ||
| eventPublisher.publishEvent(ReservationMessageEvent.of(reservation.reservationId)) |
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.
publishEvent 의 경우 비동기로 처리해도 좋을 것 같습니다 !
| const val REDISSON_LOCK_PREFIX = "LOCK:" | ||
| } | ||
|
|
||
| fun <T> lockWithTransaction(key: String, waitTime: Long, leaseTime: Long, action: () -> T): T { |
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.
확장성 있게, wait, lease, action 을 잘 분리해주신 것 같고, 이렇게 해주셨다면 common 모듈 하위에 있는게 좋은 것 같습니다 :)
| 현 서비스는 인기 영화 상영으로 인해 **동시 요청이 많을 것으로 가정**한다. | ||
|
|
||
| 경쟁이 심한 상황이기 때문에 대기 시간을 타이트하게 가져가야 한다.<br> | ||
| 앞서 설정한 leaseTime보다 더 긴 3초가 적절하다 생각된다. |
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.
버퍼 + 네트워크 지연을 고려해서 2초면 되게 좋은 것 같습니다. 전체적으로 논리적으로 잘 정리해주셨다고 생각됩니다 💯
|
@scars97 님, 고생하셨습니다 👍 👍 좋았던 점
아쉬웠던 점
Deep Dive Point
|
- as-is : 인프라 모듈에서 좌석 조회 후 상태 변경 - to-be : 도메인 모델에서 상태 변경 후 전달
[3주차] 동시성 제어 & 단계별 락 적용
📝작업 내용
🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)
JPA Lock 기능을 활용하지 못하는 문제
조회 쿼리에 락을 적용하고, 반환된 엔티티를 도메인 모델로 변환.
-> 엔티티를 도메인 모델로 변환 시 JPA 영속성이 분리되는 문제 발생.
-> 도메인 모델이 수정된 후, 엔티티로 변환해도 JPA 는 새로운 객체로 인지하여 Lock 기능 활용 X
함수형 기반 분산락 적용 안되는 문제
락 획득과 트랜잭션 시작 시점 순서가 명확하지 않아 발생한 문제로 예상되었습니다.
AOP 기반 분산락은
@Order(Ordered.HIGHEST_PRECEDENCE)을 설정하여 락을 먼저 획득한 후, 예약 로직 트랜잭션이 시작하도록 구현했습니다.함수형 기반 분산락의 경우, 트랜잭션이 이미 시작된 메서드 내부에서 동작하기 때문에
트랜잭션과 락 획득 순서가 바뀌어서 동시성 제어가 실패한 것으로 예상됩니다.
💬리뷰 요구사항(선택)
함수형 기반 분산락을 락 기능에 트랜잭션을 포함하도록 구현했습니다.
이런 경우 책임도 불분명하고, 트랜잭션이 오래 걸린다면 락이 먼저 해제될 수도 있다 생각됩니다.
개인적으로는 이 방식이 안티패턴이라 생각이 드는데,
함수형 기반 분산락을 구현할 때 락과 트랜잭션을 분리할 수 있는 방법이 있는 지 궁금합니다.
기타 사항 📌