Skip to content

Conversation

@jorippppong
Copy link

[4주차] Rate Limit 구현 및 Jacoco Report 생성

PR 제목은 변경 사항의 요약을 담아야 합니다.
예시: "[1주차] 사용자 로그인 기능 구현"

📝작업 내용

이번 PR에서 작업한 내용을 설명해주세요(이미지 첨부 가능)
주요 변경 사항을 기술해 주세요. 코드 구조, 핵심 로직 등에 대해 설명하면 좋습니다.
예시:
1 ConcurrentOrderService에 동시 주문 요청 처리 기능 추가
2 RedisLock을 사용해 주문 생성 시 동시성 제어 구현
3 동시에 여러 사용자가 주문할 경우 재고 감소가 정확히 반영되도록 로직 개선

  • 조회 API RateLimit 구현
  • (todo) 예약 API Rate Limit 구현
  • Jacoco Report 생성

🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)

잘 짰다고 생각되는 부분이 있다면 자랑해주세요😘
예시) 다수의 사용자가 동시에 같은 리소스를 업데이트할 때 재고 수량이 음수로 내려가는 데이터 불일치 문제가 발생했습니다.

  • Redis, Redisson 버전 충돌 발생
  • Lua Script에서 null이 넘어가서 비교 연산이 안되는 문제 발생
    • 값을 String으로 wrapping 한 것이 문제의 원인

🔑해당 문제를 어떻게 해결했나요? 그리고 왜 그렇게 생각했는지 이유를 남겨주세요.(자세하게 남겨주실 수록 좋습니다.)

예시) Redis SET 명령어에 NX(Not Exists)와 PX(Expire Time) 옵션을 활용해 락을 설정했습니다. 이유는~~~...

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
ex) 메서드 ㅇㅇ의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

  • 요청 단위가 글로벌 하지 않고, 핵심 로직을 테스트하기 쉽고, 비즈니스 응집적이라는 장점 때문에 AOP 를 사용해서 Rate Limit을 구현했습니다. 만약 모든 요청에 RateLimit을 적용한다고 하면 필터에 Rate Limit 로직을 추가하는것도 고민해볼 것 같은데, 리뷰어님 만의 Filter vs. Interpreter vs. AOP 에 각 어떤 로직을 넣는지에 대한 기준이 있는지 궁금합니다!
  • 저의 멀티모듈 구조는 api 모듈이 core 모듈을 의존하고, api 모듈에만 web 관련 의존성이 있습니다. 구현 시나리오 중에서 요청 횟수에 따라 특정 IP를 차단하는 기능이 있는데, AOP가 IP를 가지고 와야하는 역할이 있어서 AOP를 api 모듈에 두었습니다. 또 다른 방식으로는 api 모듈에서 IP 주소를 String 타입으로 가지고 와서 서비스 레이어의 파라미터로 전달하는 방법도 있는데, 이렇게 하게되면 Rate Limit 가 필요한 모든 메서드 마다 다 파리미터로 IP 주소를 가지고 있어야 하는데 이건 너무 비즈니스를 해치는 요소라고 생각이 들었습니다. 이러한 구조가 현재 모듈 구조에서 가장 적절한가요?
  • 앞선 이유(AOP가 api 모듈에 있어야 하는 이유) 때문에 AOP가 controller 보다 선행되서 실행이 되는데, 이런 경우에 입력 값 validation 보다 AOP가 먼저 실행이 되어서, AOP 로직의 값에 신뢰성이 떨어집니다. 이런 경우에는 어떻게 값의 신뢰성을 보장할 수 있을까요?
  • 예약API 에서 Rate Limit 로직은 비즈니스 시작 전에 요청 가능한지 확인하고, 비즈니스 로직이 완료(예매 성공)이 되면 요청 내역을 Redis에 저장하는 로직으로 구상을 했습니다. 이렇게 되면 read 와 write의 시점이 굉장히 떨어져 있는데, 이런 경우에는 Lua Script로 Rate Limit 로직을 구현하기에는 무리가 있다고 생각했습니다. 과제 시나리오에서 Lua Script를 사용해보라는 건 '동시성을 염두하고 구현해라'라는 뜻으로 받아들었는데, 지금 상황과 같이 Lua Script를 사용하기 적절하지 않은 로직에서는 읽기와 쓰기 로직을 분리해서 구현해도 문제 없을까요? 예약 API에서 어떤 식으로 Rate Limit 로직을 구현해야 할지 고민이 됩니다..!

기타 사항 📌

추가로 언급할 사항이 있다면 여기에 적어주세요.

@jorippppong jorippppong changed the base branch from main to jorippppong April 7, 2025 13:37
@ann-mj
Copy link

ann-mj commented Apr 8, 2025

@jorippppong 님, 안녕하세요~ 리뷰 시작하겠습니다.


-- 요청 내역 저장
redis.call('SETEX', key, expire_time, '1')
return 1
Copy link

Choose a reason for hiding this comment

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

lua 파일로 별도로 분리해서 관리를 잘해주셨습니다 👍

ttl = 1,
ttlTimeUnit = TimeUnit.MINUTES,
banTime = 1,
banTimeUnit = TimeUnit.HOURS
Copy link

Choose a reason for hiding this comment

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

요구사항에 맞게 깔끔하게 구현해주셧네요 👍

}

private String getClientIp() {
return request.getRemoteAddr(); // 프록시 서버는 고려 X
Copy link

Choose a reason for hiding this comment

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

ip 도 request 에서 직접 받지 않고 잘 분리해주셨습니다!


boolean allowed = strategy.isAllowed(rateLimit, getRequestUrl(), getClientIp());
if (!allowed) {
throw new CoreException(CoreErrorCode.TOO_MANY_REQUEST);
Copy link

Choose a reason for hiding this comment

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

TOO_MANY_REQUEST 예외처리도 좋습니다 :)

return type == RateLimitType.RESERVATION;
}

private String generateRequestKey(String url, String ip) {
Copy link

Choose a reason for hiding this comment

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

동일한 상영 시간대의 영화를 5분에 1번씩 할 수 있어야 하는데, screeningId 가 key 로 들어가도 괜찮지 않을까 싶습니다.

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

각 도메인 영역에 대한 테스트코드도 작성되면 좋을 것 같습니다.
Presentation Layer 에서의 통합테스트 코드도 작성해보시면 좋을 것 같습니다.

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

@jorippppong 님, 고생하셨습니다. 👍 👍
고민하신 포인트가 많이 느껴졌습니다.

좋았던 점

  • 모든 요구사항들을 꼼꼼히 확인해서 구현을 잘 해주셨습니다.
  • Aop 를 적용하여 관심사를 잘 분리해주셨습니다.
  • Lua 파일을 string 이 아닌 파일로 관리하여, 재사용성을 더 높이신 것 같아 좋습니다.

아쉬웠던 점

  • domain, presentation 에 대한 테스트 코드 작성을 하신 이후, coverage 를 더 높여봐도 좋을 것 같습니다.

질문에 대한 답변

요청 단위가 글로벌 하지 않고, 핵심 로직을 테스트하기 쉽고, 비즈니스 응집적이라는 장점 때문에 AOP 를 사용해서 Rate Limit을 구현했습니다. 만약 모든 요청에 RateLimit을 적용한다고 하면 필터에 Rate Limit 로직을 추가하는것도 고민해볼 것 같은데, 리뷰어님 만의 Filter vs. Interpreter vs. AOP 에 각 어떤 로직을 넣는지에 대한 기준이 있는지 궁금합니다!

  • data 가공이 필요한 경우에는 AOP 가 아닌 인터셉터나 resolver 로 구현을 하구
  • data 가공이 없이, 권한 체크, traffic 체크, 로그 전송 등의 로직에선 AOP 를 주로 사용합니다.

저의 멀티모듈 구조는 api 모듈이 core 모듈을 의존하고, api 모듈에만 web 관련 의존성이 있습니다. 구현 시나리오 중에서 요청 횟수에 따라 특정 IP를 차단하는 기능이 있는데, AOP가 IP를 가지고 와야하는 역할이 있어서 AOP를 api 모듈에 두었습니다. 또 다른 방식으로는 api 모듈에서 IP 주소를 String 타입으로 가지고 와서 서비스 레이어의 파라미터로 전달하는 방법도 있는데, 이렇게 하게되면 Rate Limit 가 필요한 모든 메서드 마다 다 파리미터로 IP 주소를 가지고 있어야 하는데 이건 너무 비즈니스를 해치는 요소라고 생각이 들었습니다. 이러한 구조가 현재 모듈 구조에서 가장 적절한가요?

  • 네 지금 해주신 부분이 저는 적당하다고 생각합니다. ip 를 가지고 메서드에서 넘기기에는, 비즈니스 로직을 헤칠 것 같다는 생각이 들어서 저도 해당부분을 중심으로 리뷰를 진행했었는데, 잘 구현해주셨습니다. 👍

앞선 이유(AOP가 api 모듈에 있어야 하는 이유) 때문에 AOP가 controller 보다 선행되서 실행이 되는데, 이런 경우에 입력 값 validation 보다 AOP가 먼저 실행이 되어서, AOP 로직의 값에 신뢰성이 떨어집니다. 이런 경우에는 어떻게 값의 신뢰성을 보장할 수 있을까요?

  • 해당부분은 저도 좀 더 고민을 해봐야할 것 같아요. 찾아보니 아래와 같은 방법을 적용할 수 있겠네요.
  • 방법1. AOP 에는 validation 되는 필드를 참조하지 않는 식으로 구현하면 될 것 같구요. (현재 url, ip)
  • 방법2. BindingResult 를 사용해도 좋을 것 같아요.

예약 API에서 어떤 식으로 Rate Limit 로직을 구현해야 할지 고민이 됩니다..!

  • 예약 완료 시점에 저장하고, 다시 같은 시간대 스케줄을 예약하려고 시도하는 경우 limit 을 거는거기 때문에 생각하신 대로 , 읽기-쓰기 시점이 길더라도 무방하다고 판단됩니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants