[4주차] RateLimit 적용, 테스트코드 작성#86
Open
soyoungcareer wants to merge 6 commits intohanghae-skillup:soyoungcareerfrom
Open
[4주차] RateLimit 적용, 테스트코드 작성#86soyoungcareer wants to merge 6 commits intohanghae-skillup:soyoungcareerfrom
soyoungcareer wants to merge 6 commits intohanghae-skillup:soyoungcareerfrom
Conversation
youngxpepp
approved these changes
Feb 4, 2025
youngxpepp
left a comment
youngxpepp
left a comment
There was a problem hiding this comment.
안녕하세요 소영님! 이건홍 코치입니다.
좋았던 점
- 통합 테스트를 통해 rate limit 정상 동작 검증을 잘 해주셨습니다.
- redisson과 guava를 적용해서 rate limit 기능을 적절하게 구현하셨습니다.
아쉬운 점
- guava 테스트가 제대로 진행되지 않은 것 같아 아쉽네요. guava의 RateLimiter 초기화는 어디서 이루어지죠? 초기화 코드가 없네요.
질의응답
- Q. MovieControllerTest (조회API) 테스트에서는 Google Guava로 테스트 시 429 에러가 51번째부터 찍혀야하는데,
두 번째 요청부터 계속해서 429 에러를 반환합니다. Redisson 으로 테스트 시에는 429에러를 51번째만 반환했습니다.
요청 사이 대기 시간을 늘려서 Guava를 테스트했는데, 이렇게 진행해도 되는건지 궁금합니다.
그리고 작성한 코드처럼 서버 과부하 테스트를 하는 것이 맞는지 잘 모르겠습니다.
A. guava 초기화 코드가 없어서 어떤 부분이 문제인지 인지를 못하겠네요. 관련 커밋이 있다면 저에게 알려주셔도 됩니다.
|
|
||
| // 2. 조회 API 제한 - 1분 내 50회 | ||
| if (request.getRequestURI().startsWith("/api/v1/movies") && request.getMethod().equalsIgnoreCase("GET")) { | ||
| requestCounts.put(clientIp, requestCounts.getOrDefault(clientIp, 0) + 1); |
There was a problem hiding this comment.
동일한 clientIp로 두 개의 요청이 동시에 들어왔다고 가정했을 때, 해당 코드는 정상 실행이 될까요?
동시성에 대해 다시 생각해보셨으면 좋겠습니다!
Comment on lines
+72
to
+77
| // Redisson | ||
| if (!rateLimiter.tryAcquire(1, TimeUnit.SECONDS)) { | ||
| response.setStatus(HttpStatus.TOO_MANY_REQUESTS.value()); | ||
| response.getWriter().write("현재 요청량이 너무 많습니다. 잠시 후 다시 시도해주세요."); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
클라이언트 ip별로 다르게 rateLimiter 가 존재해야 할듯 합니다.
Comment on lines
+100
to
+112
| // 예매 성공 시에만 5분 제한 적용 | ||
| if (bookingSuccess) { | ||
| // 1) Google Guava | ||
| /*reservationRateLimiters.computeIfAbsent(reservationKey, key -> RateLimiter.create(RESERVATION_RATE)); | ||
| reservationRateLimiters.get(reservationKey).tryAcquire();*/ | ||
|
|
||
| // 2) Redisson | ||
| rateLimiter.trySetRate(RateType.PER_CLIENT, 1, 5, RateIntervalUnit.MINUTES); // 5분에 1회 제한 | ||
| rateLimiter.tryAcquire(); | ||
| log.info("예매 성공 - {} 제한 적용 (5분)", reservationKey); | ||
| } else { | ||
| log.error("예매 실패 - {} 제한 적용되지 않음"); | ||
| } |
There was a problem hiding this comment.
이것도 lock 내부로 넣으면 어때요?
예매 요청이 동시에 들어온다면 trySetRate를 하기 전에 다른 요청에서 예매 로직을 수행할 수도 있겠네요!
| // lock을 획득한 요청에 대해서만 Transactional 적용하기 위해 분리 | ||
| String lockKey = "lock:screening:" + ticketRequestDTO.getScreeningId(); | ||
| boolean bookingSuccess = lockUtil.executeWithLock(lockKey, 5, 3, () -> | ||
| bookTicketsWithTransaction(ticketRequestDTO, seatNameEnums) |
There was a problem hiding this comment.
객체 내부의 메소드를 호출하면 Transactional 어노테이션이 적용되지 않아요.
https://cheese10yun.github.io/spring-transacion-same-bean/
| @Retryable(value = ObjectOptimisticLockingFailureException.class, maxAttempts = 3, backoff = @Backoff(delay = 1000)) | ||
| @Transactional | ||
| @Retryable(value = OptimisticLockingFailureException.class, maxAttempts = 3, backoff = @Backoff(delay = 1000)) | ||
| public void bookTickets(TicketRequestDTO ticketRequestDTO) { |
There was a problem hiding this comment.
bootTickets에 잠금과 rate limit 기능이 들어가 있어서 많이 복잡하네요!
추후 개선 과제로 AOP를 적용해보는 것도 좋은 방법일듯 합니다!
Comment on lines
+41
to
+57
| // 첫 번째 요청: 정상 예매 | ||
| TicketRequestDTO requestDTO = new TicketRequestDTO(screeningId, userId, List.of("E1")); | ||
| HttpEntity<TicketRequestDTO> request = new HttpEntity<>(requestDTO, headers); | ||
| ResponseEntity<String> firstResponse = restTemplate.exchange(baseUrl, HttpMethod.POST, request, String.class); | ||
| assertThat(firstResponse.getStatusCodeValue()).isEqualTo(200); | ||
|
|
||
| // 두 번째 요청: 5분 제한 적용되어야 함 (429 반환 예상) | ||
| TicketRequestDTO requestDTO2 = new TicketRequestDTO(screeningId, userId, List.of("E2")); | ||
| HttpEntity<TicketRequestDTO> request2 = new HttpEntity<>(requestDTO2, headers); | ||
|
|
||
| try { | ||
| restTemplate.exchange(baseUrl, HttpMethod.POST, request2, String.class); | ||
| System.out.println("test failed!!!"); | ||
| } catch (HttpClientErrorException.TooManyRequests e) { | ||
| System.out.println("429 Too Many Requests received as expected."); | ||
| assertThat(e.getStatusCode().value()).isEqualTo(429); | ||
| } |
There was a problem hiding this comment.
예를 들면, TicketControllerTest (예매API) 테스트에서 첫 번째 요청과 그 이후의 요청은 다른 좌석으로 요청해야하는데
이런 경우에 어떤방식으로 작성을 해야하나요? 메서드를 따로 생성해서 랜덤으로 리스트를 만들어서 요청을 보내게 해야하나요?
구체적인 시나리오로 잘 작성됐는데요?
랜덤일 필요는 없어요. 지금도 충분히 5분 제한이 잘 적용된거 같네요.
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.
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
이런 경우에 어떤방식으로 작성을 해야하나요? 메서드를 따로 생성해서 랜덤으로 리스트를 만들어서 요청을 보내게 해야하나요?
두 번째 요청부터 계속해서 429 에러를 반환합니다. Redisson 으로 테스트 시에는 429에러를 51번째만 반환했습니다.
요청 사이 대기 시간을 늘려서 Guava를 테스트했는데, 이렇게 진행해도 되는건지 궁금합니다.
그리고 작성한 코드처럼 서버 과부하 테스트를 하는 것이 맞는지 잘 모르겠습니다.
기타 질문