[3주차] 예약 API 구현 및 분산락 적용#83
Conversation
- 기존 코드는 검색 조건이 추가되면 인터페이스 변경이 필요함 - `MovieSearchCriteria` DTO를 생성해 OCP 원칙을 준수 - ModelAttribute 애노테이션으로 클라이언트 요청 값을 DTO 객체에 바인딩 - 추후 검색 조건 변경 시 DTO와 커스텀 리포지토리 구현체 코드만 수정하면 확장 가능
- 제목 검색 요청 값을 UTF-8 인코딩 기준으로 바이트 수 계산 - 스키마 상 150 바이트로 제한도니 설정과 일관된 검증을 수행
- 성능 검사를 위해 추가할 인덱스를 DDL에 작성 - `movie` 테이블에 검색 성능 향상을 위한 복합 인덱스 추가 - `screening` 테이블에서 `movie_id`를 조회할 때 풀 스캔을 방지하기 위한 인덱스 추가
- 여러 정렬 조건이 있을 경우 리스트에 모아 순서를 유지해 적용하도록 수정 - 잘못된 정렬 필드가 들어오는 것을 방지하기 위해 허용할 필드를 명시
- 현재 `title`과 `genre`로 검색 기능을 제공하는 점을 고려해 복합 인덱스 적용 - 수정된 인덱스로 실행 결과 확인 후 README에 내용 반영
- Jackson 미사용으로 불필요해진 설정을 application.yml에서 제거 - 캐싱 TTL 적용시 설정 시간을 20분으로 정하게 된 이유를 주석에 추가
- 잘못 첨부된 이미지 수정 - 보완된 설명을 추가
- `Member`: 회원 정보 관리 - `Reservation`: 회원이 생성한 예약 정보를 관리 - `SeatReservation`: 특정 예약에 대한 좌석 정보를 관리 - 좌석과 예약의 다대다 관계에서 `SeatReservation`이 중간테이블 역할을 수행
- `SeatRow`는 A부터 E까지 단일 문자로 표현되므로 타입을 Character로 개선 - 타입 변경으로 관련 메서드 코드 수정 - equals 메서드에서 Objects.equals 사용으로 null 안전성 확보 - Validation Layer에서 호출할 수 있도록 Getter 추가
- 예약 생성 과정에서 데이터 무결성 보장하기 위한 검증 로직 구현 - 회원 객체, 예약 요청 데이터, 좌석 선택 관련 유효성을 검증 - 검증 실패 시 CustomException을 던질 때 필요한 ErrorCode 추가
- 추가된 엔티티를 기반으로 저장소 포트 및 어댑터 구현 - 불필요한 주석 제거
- 예약 요청 및 응답 시 데이터를 전달할 DTO 생성
- 비즈니스 로직 상 비어 있는 리스트가 문제될 수 있는 것에 null 체크 - ErrorCode 및 메시지를 추가해 명확한 예외 메시지 제공
- 예약 서비스에서 Validation 서비스를 직접 참조하지 않도록 구조 개선
- Validation 포트를 사용해 예약 생성 서비스 구현
- 상속 및 구현 대상이 잘못 기재된 내용을 수정
- 코드 가독성을 위한 개행 추가 및 줄바꿈 수정
- `Member`, `Reservation`, `SeatReservation` DDL 추가
- 클라이언트 요청 값 검증 로직을 `validateReservationRequest` 메서드에 분리 - 컨트롤러에서 해당 메서드를 통해 기본 유효성을 체크하고 Early-Ex을 유도 - 서비스는 DB 데이터를 기반으로 비즈니스 로직과 데이터 일관성 검사 책임만 갖도록 개선
- CustomException이 개별 작성된 메시지를 반환할 수 있도록 수정 - 기본으로 작성된 예외 메시지로 일괄 처리되던 문제 해결
- data.sql에 회원 데이터 100개 삽입 쿼리 추가 - 좌석 예약 요청을 테스트할 .http 파일 생성
- 메시지 전송을 위한 포트와 구현체 클래스 생성 - FCM 연동 없이 로그 출력으로 대체 - Thread.sleep(500) 적용하여 메시지 발송 처리 시간 시뮬레이션 - 예약 완료 메시지에 사용자명, 좌석 번호, 영화 정보 포함 처리
- 상영 정보 및 회원 검증 로직은 현재 제공되는 기능에 사용되지 않아 삭제 처리
- 스레드 100개가 동일한 좌석을 예약 동시에 예약하는 테스트 작성 - 동시성 문제가 발생해 데이터 정합성이 유지되는지 확인하는 목적 - 테스트 코드에 필요한 관련 메서드를 추가 구현 - 현재는 락을 적용하지 않았기 때문에 테스트가 실패
- 사용하지 않는 fetch type 및 import문 제거
- `ReservationService`에서 좌석 예약 전 잠금 후 검증 로직 구현 - 비관적 락을 적용한 `existsByScreeningAndSeat` 메서드 사용 - 비관적 락은 동일한 트랜잭션 내에서만 유효하므로 `@Transactional` 추가 - 좌석 조회 시 즉시 잠금을 적용해 다른 트랜잭션에서 동시에 예약하지 못하도록 방지 - 여러 좌석 예약 요청 시 `validateSeatsExists`를 활용한 사전 검증 보완
- `Create`메서드는 핵심 흐름만 확인하고 단일 책임 원칙 준수 - `getScreening()`, `getMember()`: 정보 DB 조회 로직 - `validateReservationConstraints()`: 검증 로직을 통합 - `saveReservationAndSeats()`: 최종 예약 및 좌석 저장
- 좌석 예약 관리를 위해 `ScreeningSeat` 및 `ReservedSeat` 테이블 추가 - `ScreeningSeat`에 version 필드를 추가로 낙관락 적용 동시성 제어 - `ReservedSeat`으로 예약 정보와 좌석 정보를 연결 - 변경된 구조 README에 반영
- DB 변경에 따라 리포지토리 어댑터 및 포트 수정
- 추가된 테이블에 대한 DDL 작성
- `ReservationService`에 비관락 대신 낙관락 적용
- 변경된 DB 구조와 낙관락 메커니즘에 맞게 테스트를 수정 - 정상적으로 동시성이 제어되는 것을 확인
- 분산 락 적용을 위한 의존성 추가 - @distributedlock 애노테이션을 이용해 AOP 기반 분산 락 적용 - `screeningId` 및 `seatId`를 조합해 락 키를 설정해 충돌 방지 - 트랜잭션 실행 시간이 500ms ~ 750ms 으로 관찰됨 - 이를 기반으로 leaseTime(잠금 유지 시간)을 약 2배인 1초로 설정 - waitTime(대기 시간)은 leaseTime의 2배인 2초로 설정 - 성능 테스트 결과 README에 반영
- 직접 분산락을 호출해 특정 코드 블록에만 적용하는 방식으로 변경 - `saveReservationAndSeats` 메서드 실행 전에 락을 획득 - 재시도 로직 추가 후 테스트 시 응답 속도가 더 느려져 구현에서 제외 - 성능 테스트 시 응답 시간에서 약간 개선됨을 확인
|
@zhdiddl 안녕하세요 진아님 ~ 3주차 열심히 구현해주셔서 감사합니다 ! 리뷰 진행하겠습니다 ~ |
| // 함수형 분산 락을 특정 메서드에 적용 | ||
| String lockKey = "seat_reservation:" + request.screeningId() + ":" + request.seatIds(); | ||
| long waitTime = 2000; | ||
| long leaseTime = 1000; |
There was a problem hiding this comment.
평균 작업시간(500ms) �기준 고려해서 2배로 설정하신 것 같은데, GC가 발생하는 상황이라던지 긴 시간 대기가 발생할 수 있어 3초~5초 정도로 하는건 어떨지 의견드려봅니다.
There was a problem hiding this comment.
해당 부분 반영해 leaseTime을 5초로 수정했습니다! waitTime은 leaseTime의 2배로 잡으면 10초가 되어 다소 길어지는 데, leaseTime과 동일하게 5초로 설정하는 것도 괜찮을까요?
There was a problem hiding this comment.
leaseTime 이 5초이긴 해서 보다 여유롭게 7초 정도로 해둬도 좋을 것 같습니다.
| } | ||
|
|
||
| @Override | ||
| public <T> T executeWithLock(String key, long waitTime, long leaseTime, Supplier<T> task) { |
|
|
||
| private final MessageServicePort messageServicePort; | ||
|
|
||
| @DistributedLock(key = "'seat_reservation:' + #request.screeningId + ':' + #request.seatIds.toString()") |
There was a problem hiding this comment.
@Transacational 메서드로 선언되어 있는데, 분산락을 적용한경우
동시성 이슈가 발생할 수 있을 것 같습니다.
- 락 해제 시점과 트랜잭션 커밋 시점이 일치하지 않을 가능성이 있습니다.
- Thread A가
@DistributedLock을 통해 분산락을 획득하고, 트랜잭션을 시작합니다. - Thread A가 트랜잭션 내에서 데이터베이스 작업을 수행합니다.
- 작업이 끝난 후, Thread A가 분산락을 해제합니다. (이 시점에서 트랜잭션은 아직 커밋되지 않았습니다.)
- Thread B가 동일한 키에 대해 분산락을 획득하고, 같은 좌석에 대해 작업을 시작합니다.
- Thread A의 트랜잭션이 아직 커밋되지 않은 상태라면, Thread B는 Thread A의 작업 결과를 보지 못하고 중복된 데이터 작업(예: 좌석 중복 예약)을 시도할 수 있습니다.
트랜잭션을 분산락 안으로 이동하면 좋을 것 같습니다 :)
There was a problem hiding this comment.
락을 먼저 획득한 다음 트랜잭션이 시작되도록 설정해야 하는군요.
피드백 주신 부분에 대해 알아보며 많은 공부가 되었습니다. 감사합니다!
분산락을 함수로 적용한 경우 transactionTemplate를 사용해 분산락 내부에서 트랜잭션이 실행되도록 리팩토랭했습니다.
분산락을 AOP로 처리하는 경우에는 서비스 클래스대신 DistributedLockAspect 클래스 로직 안에서 적용한다고 보면 될까요?
There was a problem hiding this comment.
Reservation reservation = saveReservationAndSeats(screening, member, requestedSeats);
return ReservationResponseDto.fromEntity(reservation);
넵! 요 로직을 메서드로 만들고, Transactional 로 감싸주면 좋을 것 같습니다.
구조적으로는 잘 구현해주신 것으로 보입니다. 낙관락을 제어해보기 위함이었다고 생각되구요. |
OptimisticLockingFailureException 에 대한 예외를 잡아서 별도의 409 코드로 처리하면 될 것 같습니다. 현재는 500에러가 뜨고 계신 것이지요? |
|
|
||
| @DisplayName("100개 스레드가 동일한 좌석을 동시에 예약할 때 중복 예약이 발생하지 않는다.") | ||
| @Test | ||
| void shouldPreventConcurrentSeatReservation() throws InterruptedException { |
|
|
| private static final int MAX_SEATS_PER_SCREENING = 5; | ||
|
|
||
| public void validateReservationRequest(Long screeningId, Long memberId, List<Long> seatIds) { | ||
| if (Objects.isNull(screeningId)) { |
There was a problem hiding this comment.
if(screeningId == null)
저는 보통 if 문 안에서는 Objects.isNull을 잘 사용하지는 않습니다.
이부분은 보통 stream 에서 주로 사용합니다.
ex) .filter(Objects::isNull) // null만 걸러냄
There was a problem hiding this comment.
추가 메서드 호출이 불필요한 경우라고 봐야겠네요. 작은 부분도 더 신경 써야겠습니다. 감사합니다!
| config.useSingleServer() | ||
| .setAddress("redis://localhost:6379") | ||
| .setConnectionPoolSize(10) | ||
| .setConnectionMinimumIdleSize(5); |
There was a problem hiding this comment.
연결 타임아웃 설정, 재시도 횟수 등도 설정 할 수 있으니 나중에 필요하실 때 한번 찾아보시는 것을 추천드립니다 👍
|
@zhdiddl 님 좋았던 점
아쉬운 점
추가적인 의견
|
- GC 실행 시 STW로 락이 만료될 가능성을 고려해 leaseTime을 5초로 수정 - 트랜잭션 실행 시점을 락 내부로 이동해 락 종료 전에 트랜잭션이 먼저 커밋되도록 수정 - 낙관락 충돌 여부 확인을 위해 예약 생성 로직에 로그를 추가 - 좌석 상태 변경 시 트랜잭션이 종료 전에 버전 업데이트가 DB에 반영되도록 flush 호출 추가
- 저장소에 필요한 메서드를 남기고 사용하지 않는 메서드 제거 - 불필요한 import문 제거 - 코드 가독성을 위한 빈 줄 추가 및 메서드명 변경
- 낙관 락 충돌 예외 핸들러 추가해서 클라이언트에게 일관된 응답 반환 - CustomException 처리 시 필요한 에러코드 추가
- 락 획득 시도, 성공, 실패 로그를 추가해 모니터링 강화 - 예외 처리에 CustomException 적용해 일관된 예외 관리 구현 - 락 해제 시 예외가 발생할 경우 강제 해제를 실행해 데드락 상황 방지 - 롬복 애노테이션 사용으로 보일러플레이트 코드 제거
- if 문에서 Objects.isNull() 대신 == null 사용 - 불필요한 메서드 호출을 줄여 성능 최적화 - 코드 스타일을 일관되게 유지하여 가독성 향상
- Redis 명령 실행 타임아웃을 5초로 설정하여 지연 응답에 대한 예외 처리 - Redis 서버 연결 시 타임아웃을 5초로 설정하여 연결 지연을 방지 - 최대 3번의 재시도 및 재시도 간격을 1.5초로 설정하여 일시적인 연결 문제 해결
- 스프링 부트를 실행하는 모듈에 bootJar 생성 설정 - 캐시 관련 설정에 잘못된
- Redis를 캐시 저장소로 사용하기 위한 설정 - application.yml에 cache.type 추가
3 participants
제목(title)
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.