[4주차] RateLimit 적용 및 테스트 코드 작성#96
Conversation
3rdweek to dev
Refactor 3rdweek
|
안녕하세요 우준님 ! 이번주도 고생 많으셨습니다! |
| return distributedLockFunction.executeFunctionalLock( | ||
| key, | ||
| waitSeconds, | ||
| leaseSeconds, | ||
| () -> { | ||
| Reservation reservation = initReservation(request); | ||
| List<Seat> seats = getSeats(request); | ||
|
|
||
| List<ScheduleSeat> scheduleSeats = getScheduleSeats(request, seats); | ||
| checkDoubleBooking(scheduleSeats); | ||
|
|
||
| List<ReservationSeat> reservationSeats = toReservationSeats(reservation, seats); | ||
| checkReservationPolicy(reservation, reservationSeats, MAX_SEATS_PER_RESERVATION); | ||
|
|
||
| reservation.setReservationSeats(reservationSeats); | ||
| scheduleSeats.forEach(scheduleSeat -> scheduleSeat.setReserved(true)); | ||
|
|
||
| return ReservationResponse.from(reservationRepository.reserve(reservation)); | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
현재 이 부분에 함수형 분산락을 걸어주셨는데요.
락이 필요 없는 데이터 조회 및 검증 로직이 포함되어 있다면, 락을 걸기 전에 실행하는 것이 좋습니다.
- 좌석 정보를 조회하는 부분(getSeats, getScheduleSeats)은 락을 걸지 않아도 되는 연산이므로, 락을 걸기 전에 수행하는 것이 좋습니다.
- DB 업데이트(쓰기 작업)만 락이 적용된 상태에서 실행하도록 수정하면 락 점유 시간이 줄어들어 성능이 개선될 수 있습니다.
- 임계영역을 최소화하면 대기 시간이 감소하여, 성능 테스트 결과에도 영향을 미칠 수 있어요!
| return ReservationResponse.from(reservationResult); | ||
| } | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
현재 메소드 전체에 Transactional을 사용해 주셔서 락을 획득하기 전에 트랜잭션이 시작될 가능 성이 있습니다!
트랙잭선을 락 내부에서 실행하도록 변경해 주는 것이 좋습니다!
| throw new RuntimeException("Lock interrupted: " + e.getMessage()); | ||
| } finally { | ||
| if (lock.isHeldByCurrentThread()) { | ||
| lock.unlock(); |
There was a problem hiding this comment.
락을 해제할 때에도 exception이 발생할 수 있는데요. 이때 처리를 따로 해주지 않는다면 락이 풀리지 않을 수도 있겠죠.
사용하고 계신 Redisson에서는 IllegalMonitorStateException이 발생할 수도 있어서요.
try-catch 로 안전하게 해제 하시는 것을 추천 드립니다.
| public interface ReservationRateLimitChecker { | ||
|
|
||
| void oneReservationPerFiveMinutes(String key); |
There was a problem hiding this comment.
이 부분에서 순환 참조를 해결하기 위해 인터페이스를 도메인 모듈에 추가 하신 것으로 보이는데요.
현재 적용한 방식은 아키텍쳐를 흔들지 않고 도메인과 인프라의 경계를 잘 구분하신 것으로 보이네요!
| } | ||
|
|
||
| @Test | ||
| public void testRateLimit() throws ServletException, IOException { |
There was a problem hiding this comment.
RateLimit 테스트를 잘 구현해 주셨네요!
시간이 되신다면 동시 요청이 있을때의 RateLimiter 적용이 정상적으로 작동하는지 확인하는 테스트도 추가하시는 것이 좋아요!
| // 51번 째 요청 | ||
| response = new MockHttpServletResponse(); | ||
| rateLimitFilter.doFilter(request, response, filterChain); | ||
| assertEquals(429, response.getStatus(), "51st request should be blocked"); |
There was a problem hiding this comment.
현재 RAtomicLong을 사용하고 계신데, 개별 키 값을 관리해야 하기 때문에 유지보수 측면에서 불편함이 있을 수 있습니다.
이를 개선하기 위해 RMapCache를 사용하면 한 곳에서 IP 요청 수를 관리할 수 있어 더 나은 방법으로 보입니다.
다만, Redis CLI에서 TTL을 직접 조회하기 어렵다는 단점이 있는데, 이를 보완하기 위해 TTL을 별도로 저장하는 RMapCache를 추가하면 TTL 조회가 가능할 것 같습니다.
좋았던점
아쉬운점
리뷰 포인트-> 코드리뷰에 작성해 두었습니다. 😄 기타 질문
4주 동안 정말 고생 많으셨습니다! |
2 participants
조회 및 예매 로직에 RateLimit 적용, 조회 RateLimiting 테스트 작성, Jacoco 설정 추가
작업 내용
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
기타 질문