Skip to content

[ 4주차 ] Ratelimit 구현 및 테스트 케이스 작성#97

Merged
soonhankwon merged 16 commits intohanghae-skillup:futuremaker019from
futuremaker-projects:dev
Feb 4, 2025
Merged

[ 4주차 ] Ratelimit 구현 및 테스트 케이스 작성#97
soonhankwon merged 16 commits intohanghae-skillup:futuremaker019from
futuremaker-projects:dev

Conversation

@futuremaker019
Copy link

제목(title)

[ 4주차 ] Ratelimit 구현 및 테스트 케이스 작성


작업 내용

작업내용 입니다.

  • 조회 API의 비정상적인 사용 패턴 감지 및 시스템 보호 기능 개발
    • 단일 서버 환경에서 Rate Limit 구현
    • 분산 서버 환경에서 Rate Limit 구현
  • 예약 API의 유저당 같은 시간대의 영화 5분간 재예약 방지 기능 개발
    • 단일 서버 환경에서 Rate Limit 구현
    • 분산 서버 환경에서 Rate Limit 구현

작업은 인터셉터를 이용해서 Redis 캐시를 이용해 개발했습니다.

  1. 단일 서버 환경의 경우, 과제에 주어진 Google Guava를 이용하여 1초에 2회 이상의 요청이 들어오면 Too many Request 예외가 발생하도록 하였으며,
  2. 분산 서버 환경의 경우, Redisson의 Rate Limit 기능을 이용하여 구현하였습니다.

테스트 케이스를 작성했지만 부족한 부분이 있어, 다음주까지 보충하여 제출하겠습니다. (lua script 및 Jacoco 포함)

발생했던 문제와 해결 과정을 남겨 주세요.

  • 1분 내에 50번의 요청에 대한 패턴 감지에는 Guava의 사용으로는 적절치 않아, Redis의 캐시를 이용해 IP를 차단할 수 있도록 개발했으며, 1시간의 TTL을 주어 비정상적인 요청을 한 IP를 차단하도록 했습니다.
  • Redisson의 RRateLimiter 를 api가 직접 받아 사용하려고 했지만, 모듈간 순환참조 문제 발생으로 인해, 부득이 하게 domain에 RatelimiterHandler를 두고, movie-infa의 redis에서 인터페이스를 구현하여 사용하도록 변경했습니다.

이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.

  • Guava 적용에 많은 시간이 걸렸습니다. 1분내에 50번의 요청을 감지하는게 아닌 초 단위의 요청감지로 bucket의 토큰을 사용하는 방식이라 어떻게든 요구사항에 맞게 조절하려고 시도해보고 테스트 케이스에도 Thead.sleep(1000) 을 주어 시도 했지만 원하는 테스트 성공을 확인하지 못했습니다. 그래서 Redis로 IP를 캐시 처리하도록 변경했는데 이 부분은 멘토님께서 어떻게 생각하시는지 궁금합니다.

리뷰 포인트

  • 테스트 케이스 작성시, 각 레이어별로 해당하는 테스트를 작성해야 하는지 궁금합니다. 차주의 예약 API 구현시 의존성으로 인해 직접 storage에서 테스트 케이스를 작성했는데 이런식의 접근도 괜찮은건지요?

- 분산락이 해제된 후, DB 트랜젝션의 lost update 상황을 방지하기 위해 적용
@soonhankwon
Copy link

안녕하세요 구현님 😃
4주차 열심히 참여해주셔서 감사합니다. 리뷰 진행하겠습니다 :)

}

// Redis에서 요청 횟수 증가 및 초과 체크
if (rateLimiterHandler.isRequestLimitExceeded(ip)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis를 활용해서 rateLimter 구현을 적절하게 잘 해주신점 좋네요 👍

  • 알맞은 응답(429)를 반환하는 점도 좋습니다.

import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@SpringBootTest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API에 대한 통합테스트 작성하신점 좋습니다 👍

Thread.sleep(1000);
} catch (Exception e) {
// 예외가 발생하면 계속해서 실패 메시지 출력
System.out.println("Request " + (i + 1) + " failed due to: " + e.getMessage());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.out 보다는 로깅을 활용하는편이 성능상 좋습니다 :)


@DisplayName("")
@DisplayName("좌석 5개 이상 예약 시도 시, 예외를 발생시킨다.")
@Test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Domain에 대한 Unit Test 작성 좋습니다 👍

.header("X-Forwarded-For", ip)
)
.andDo(print())
.andExpect(status().isTooManyRequests()); // 요청이 초과되어 429 Too Many Requests 응답

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ratelimit가 적용된 "실패해야하는 부분"에 대한 테스트 작성되있어서 좋습니다 👍

private final ReservationMessageSender messageSender;

@Async
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이벤트리스너활용 & 비동기처리로 메서지전송 로직과 결합도를 낮춰주신점 좋네요 👍

@soonhankwon
Copy link

좋았던 점

  • API 구현(응답 스펙)이 대체적으로 적절해서 좋았습니다.
  • Ratelimit를 잘 구현하고 요구사항에 맞게 적용하셨습니다.
    • IP 별로 제한하신 부분 및 세심하게 제한하신 부분 좋습니다.
  • Ratelimit 적용시 적절한 HTTP 상태코드를 응답(429)하는점 이 좋습니다.
  • Presentation Layer에서 통합 테스트를 잘 작성하시고, RateLimit에 대한 테스트를 해주셨습니다.

아쉬웠던 점

  • 테스트 커버리지에 대한 내용 및 테스트 코드가 좀 더 보완되면 좋겠습니다.
    • 다음주에 보완해서 제출하시면 좋을것 같습니다.

리뷰 포인트

  • 버킷토큰 테스트 케이스 실패, Redis로 IP를 캐시 처리하도록 변경

    • Redis에 IP로 rateLimit를 구현한것은 좋은 선택같습니다. 결국 분산 Ratelimit를 현업에서는 주로 사용하게됩니다.
    • 다만 의문인점은 “버킷토큰”은 단순하게 “알고리즘”이라 세부적으로 구현하면 1분내 50번 요청을 막을 수 있습니다.
    • 해당 부분은 왜 실패하는지 좀 더 확인이 필요해보이네요 :)
  • 테스트 케이스 작성시, 각 레이어별로 해당하는 테스트를 작성해야 하는지?

    • 레이어별로 해당하는 테스트가 작성되있으면 유지보수에 좋습니다.
    • 다만, 일부 통합테스트 및 Medium 테스트의 경우 특정 레이어(주로 모든 의존성을 가지는)에서 진행하는 경우도 읶겠습니다.
    • “개인적인 의견”으로 테스트가 어느 레이어에 있는지?, 테스트 커버리지 100% 이런것은 개발자의 노력에 비해 큰 생산성을 보이지 않습니다.
    • 의미있는 통합테스트 및 버그가 발생한 부분에 대한 테스트코드 등을 작성하는데 주안점을 두면 좋겠다고 의견을 드립니다.

@soonhankwon soonhankwon merged commit 0a65bdf into hanghae-skillup:futuremaker019 Feb 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants