Open
Conversation
This reverts commit b326a56.
youngxpepp
approved these changes
Feb 4, 2025
There was a problem hiding this comment.
안녕하세요 연주님! 이건홍 코치입니다.
좋았던 점
- 사용자 요청에 대한 검증을 꼼꼼하게 잘해주셨습니다. 좌석이 연달아 들어왔는지? 최대 좌석 개수를 넘진 않는지? 등등...
- 분산락을 통해서 티켓 예약에 대한 동시성을 잘 해결해주셨습니다. 특히 multi lock 👍
아쉬운 점
- 만들어주신 예약 로직에 조금의 엣지 케이스가 있습니다. 리뷰 확인 부탁 드릴게요!
질의응답
- Q. 비관적 락이 적용이 안되는 원인을 찾아보니, 격리 수준이 REAPEATABLE READ 라, 트랜잭션이 커밋 이전 데이터를 읽어서였습니다. 따라서, 트랜잭션 격리 수준을 READ_COMMITED로 수정해주었는데, 수정해도 괜찮을까요?
A. 비관적 락과 트랜잭션 격리 수준은 크게 관련이 없습니다. 적용이 안된다는걸 어떻게 아셨고, 어떤 시나리오로 진행하셨는지 궁금하네요. - Q. 함수형 분산락이 적용된 범위가 올바른지 궁금합니다. 예매 동작만 임계 영역으로 설정하고 싶었지만, 좌석 예매 특성 상 A1, A2, A3의 락을 모두 획득한 후, 진행해야함에 따라 for문 전체를 감싸게 되었습니다.
A. 네 적절하게 감싸주셨어요. 내가 예매하려는 좌석들이 임계 영역 입니다. - Q. ReservationService에서 다른 도메인 Repository를 많이 참조하고 있는데, 괜찮나요? 유저 검증, 좌석 검증, 영화 검증 등의 로직이 필요해서 다른 도메인들을 참조해야하는 상황에서는 실무에서 어떻게 하는지 궁금합니다.
A. 엄격하게 하려면 서비스를 참조하는게 맞지만, 비지니스 로직이 없는 단순 조회의 경우 repository를 사용해도 상관 없다고 생각합니다.
Comment on lines
+88
to
+106
| private boolean areSeatsAdjacent(List<String> seats) { | ||
| for (int i = 0; i < seats.size() - 1; i++) { | ||
| String current = seats.get(i); | ||
| String next = seats.get(i + 1); | ||
| if (!areSeatsNextToEachOther(current, next)) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| private boolean areSeatsNextToEachOther(String current, String next) { | ||
| char currentRow = current.charAt(0); | ||
| char nextRow = next.charAt(0); | ||
| int currentSeat = Integer.parseInt(current.substring(1)); | ||
| int nextSeat = Integer.parseInt(next.substring(1)); | ||
|
|
||
| return currentRow == nextRow && nextSeat == currentSeat + 1; | ||
| } |
There was a problem hiding this comment.
연속된 좌석이 들어오는지 확인하는 로직이네요? 꼼꼼하게 잘 검증해주셨어요ㅎㅎ
하지만, A1, A3, A2 와 같이 순서가 바뀌어서 들어온다면 어떻게 하실건가요?!
사용자의 요청을 100퍼센트 신뢰해선 안됩니다!
Comment on lines
+58
to
+60
| if (reservationRequest.seats().size() > MAX_RESERVATION_PER_PERSON) { | ||
| throw new IllegalArgumentException(RESERVATION_MUST_BE_UNDER_MAX); | ||
| } |
There was a problem hiding this comment.
‘범죄 도시’ 영화의 상영 시간표가 08:00(~10:00), 12:00(~14:00) 처럼 2건이 있을때, User-A 는 08:00 타임에 최대 5개의 좌석을 예약할 수 있습니다. 5개 좌석을 A1, A2 1번, C1, C2, C3 1번으로 총 두 번으로 나눠서 진행 가능합니다.
과제 요구사항입니다.
한 타임에 최대 5개 좌석을 예매할 수 있지만, 예약 1회에 제한된 좌석 개수가 있네요~
Comment on lines
+72
to
+73
| Seat findSeat = seatRepository.findByPositionAndScreeningId(position, screening.getId()).orElseThrow(() -> | ||
| new IllegalArgumentException(SEAT_NOT_FOUND)); |
There was a problem hiding this comment.
클라이언트에 좌석 id는 노출 안될까요?
클라이언트에서 좌석 id를 받을 수 있다면, join을 2번하는 경우는 막을 수 있을거 같아요!
Comment on lines
+23
to
+29
| List<RLock> locks = seatIds.stream() | ||
| .map(seatId -> redissonClient.getLock("lock:screening:" + screeningId + ":seat:" + seatId)) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| RLock multiLock = new RedissonMultiLock(locks.toArray(new RLock[0])); | ||
|
|
||
| boolean lockAcquired = false; |
|
|
||
| private final RedisLockTransaction redisLockTransaction; | ||
|
|
||
| public <T> T executeWithLockAndReturn(Supplier<T> action, Long screeningId, List<String> seatIds, long waitTime, long leaseTime) throws InterruptedException { |
There was a problem hiding this comment.
waitTime과 leaseTime이 있어서 좋네요!
해당 파라미터가 초단위라는걸 알 수 있게 하면 어때요?
Comment on lines
+66
to
+68
| if (reservationRepository.getReservationCount(findUser.getId(),screening.getId()) + reservationRequest.seats().size() > MAX_RESERVATION_PER_PERSON ) { | ||
| throw new IllegalStateException(RESERVATION_MUST_BE_UNDER_MAX); | ||
| } |
There was a problem hiding this comment.
만약 어뷰징 사용자가 요청 A, B, C를 정확하게 동시에 보내면 어떻게 될까요?
위 검증 과정은 제대로 동작할까요?
- 08:00 히트맨 2
- 요청 A: 좌석 A1 A2 A3 예약
- 요청 B: 좌석 B1 B2 예약
- 요청 C: 좌석 C3 C4 C5 예약
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 내용
3주차 제출 때, 단계별 Lock 적용에 실패해서 적용 후 다시 제출합니다.
발생했던 문제와 해결 과정을 남겨 주세요.
질문