refacator: API 의존성 관리 로직 개선 #910

hyeon0208 · 2024-11-15T11:09:18Z

🚩 연관 이슈

close #900

📝 작업 내용

문제 상황

List로 RouteClient를 관리하고 있기 때문에
첫 번째 RouteClient가 항상 실패하는 상황에서도 매번 호출 후 다음 RouteClient로 부터 요청 응답을 받는다.
이로 인해 다음과 같은 문제가 발생한다.

반복적인 쌓이는 너무 긴 에러 로그
타임아웃 대기 시간만큼 불필요한 지연 발생
불필요한 연결 및 타임아웃 처리를 위한 시스템 리소스 낭비

얻고자한 점

Redis를 도입해 에러가 발생한 요소에 TTL을 설정해 쿨다운을 적용하는 서킷 브레이커 패턴을 적용하여 아래와 같이 개선하고자 함.

쿨다운 동안만 다른 RouteClient에게 요청 응답을 받도록 개선.
분산 된 서버들도 일관되게 동작

적용 방식 흐름

쿨다운 관리
- 에러 발생 시 장애 횟수를 1씩 카운팅
- 30분 내에 총 3번의 RouteClient 에러 발생 시 즉시 3시간의 쿨다운 시작
- 쿨다운 중이 아닌 클라이언트만 필터링하여 사용

로컬과 Dev, Prod

로컬에서는 기존에 사용하던 MySQL과 같이 Docker Compose 파일에 redis 설정을 추가해 사용가능하도록 했습니다.
dev와 prod 환경은 AWS의 Elasticache로 Redis를 추가하여 연결해주었어요.

cache.t4g.micro로 생성했고 저의 프로젝트 남은 기간동인 14일 동안 사용한다면 약 11,250원이 나올걸 봐서
저희 지원 제한 요금인 70달러는 초과하지 않을 것 같아요

🏞️ 스크린샷 (선택)

🗣️ 리뷰 요구사항 (선택)

block(쿨다운)에 대한 TTL

현재 3회 실패시 3시간의 TTL을 걸어 해당 시간동안 다른 RouteClient를 사용하도록 적용했어요
첫 번째 RouteClient인 Odsay의 경우 주로 에러를 뱉는 상황이 API 제한량(일/1,000)을 초과할 경우일텐데
일시적인 장애 문제도 고려, Google에는 하루라는 단위가 성립안됨을 고려했고
보통 위와 같은 이유로 장애가 발생하면 긴 시간동안 사용을 할 수 없어 3시간 단위로 확인할 하도록 설정했는데요
다른 분들은 어느 정도 TTL이 적절하다고 생각하시나요 ?

eun-byeol

카키 고생하셨어요! 깔끔하게 구현해주셨네요👍 덕분에 레디스와 서킷 브레이커 패턴 학습할 수 있었어요.

다른 분들은 어느 정도 TTL이 적절하다고 생각하시나요 ?

3시간 쿨타임도 합리적이라고 생각해요. 근데 딱 3시간이라는 기준의 근거가 궁금하네요🤔
전 가능하다면, 동적으로 쿨타임을 정하면 좋을 것 같아요.
가장 짧은 갱신 주기인 오디세이에 맞춰, 당일 23:59:59까지 쿨타임이 적용되도록요.

eun-byeol · 2024-11-19T05:09:50Z

backend/src/main/java/com/ody/route/service/RouteClientCircuitBreaker.java

+    public void recordFailCountInMinutes(RouteClient routeClient) {
+        String failClientKey = RouteClientKey.getFailKey(routeClient);
+        int failCount = redisTemplate.increment(failClientKey);
+        redisTemplate.expire(failClientKey, FAIL_MINUTES_TTL);


[질문]
실패 카운트가 될 때마다, FAIL TTL이 갱신되는 구조로 보여요. 매번 갱신하는 이유가 있을까요?

30분 내에 3번의 에러가 발생했을 때에만 격리 시키는 것을 의도했습니다!

eun-byeol · 2024-11-19T05:14:07Z

backend/src/main/java/com/ody/common/redis/CustomRedisTemplate.java

+
+@Component
+@RequiredArgsConstructor
+public class CustomRedisTemplate extends RedisTemplate<String, String> {


[질문]
value는 Integer 타입이어도 될 것 같은데, String으로 한 이유가 있나요?
increment, getKeyCount 메서드는 범용적으로 사용되긴 어려울 것 같아요.

[제안]
CustomRedisTemplate 보다 RouteClientRedisTemplate 처럼 더 구체화 하는건 어떤가요?

Redis에 모든 값들이 String으로 저장되어 형변환 없이 String으로 가져온 뒤 원하는 타입으로 변경하도록 의도했습니다!

카운팅, 카운트 조회 기능이 다른 도메인에서도 사용될 여지가 있다고 생각했었는데
각 도메인마다 가져오고 싶은 타입이 다를 수 있으니 구체화해도 좋을것 같네요!

eun-byeol · 2024-11-19T05:21:18Z

backend/src/main/java/com/ody/route/service/RouteClientCircuitBreaker.java

+    }
+
+    private void block(String blockKey) {
+        redisTemplate.opsForValue().set(blockKey, "1");


[제안]
1도 상수로 빼면 더 좋을 것 같아요!

BLOCK으로 상수화했습니다!

eun-byeol · 2024-11-19T05:25:37Z

backend/src/main/java/com/ody/route/service/RouteClientCircuitBreaker.java

+    }
+
+    private void clearFailCount(String failCountKey) {
+        redisTemplate.unlink(failCountKey);


unlink 이런것도 있었네요! delete와 달리 비동기로 삭제되군요

eun-byeol · 2024-11-19T05:34:16Z

backend/src/main/java/com/ody/route/service/RouteService.java

+                routeClientCircuitBreaker.recordFailCountInMinutes(routeClient);
+                routeClientCircuitBreaker.determineBlock(routeClient);


[제안]
서버 에러인 경우만 실패 카운트를 기록하는 게 더 좋을 것 같아요.
해당 스코프로 들어오는게 클라이언트 에러가 될 가능성도 있습니다. (ex. Dto에서 수도권 검증에 통과한 위경도 좌표가, 오디세이에서는 지원하지 않는 좌표일 수 있음)

현재로썬 클라이언트 에러도 카운팅하고 있네요
외부 API 응답에 장애가 발생했을 때 던지는 에러인 OdyServerErrorException를 잡아 카운팅하도록 수정했습니다!

eun-byeol · 2024-11-19T05:35:58Z

backend/src/main/resources/application-dev.yml

+    redis:
+      host: ody-redis-001.ody-redis.vd1e5e.apn2.cache.amazonaws.com


[제안]
레디스도 비밀번호 설정 해주면 어떨까요? 예전 프로젝트에서 비밀번호 미설정으로 레디스 해킹당한 적이 있습니다ㅎㅎ

비밀번호 설정이 필요할 것 같네요!
로컬 Redis 비밀번호는 도커 컴포즈 파일로 확인가능하고
dev, prod Redis 비밀번호는 노션 계정 페이지에 첨부했습니다!

redis-cli 접속 시 AUTH 비밀번호 명령어로 인증할 수 있습니다

eun-byeol · 2024-11-19T05:45:25Z

backend/src/test/java/com/ody/common/redis/CustomRedisTemplateTest.java

+
+    private static final String TEST_KEY = "test";
+
+    @DisplayName("key의 counter를 증가시킨다.")


[제안]
키가 없는 상황(0 -> 1), 키가 있는 상황(1 -> 2) 로직이 다른데, 둘다 검증하면 어떤가요?

키가 없을 때 카운팅은 +1, 있을 때도 카운팅은 +1 이라서 같다고 생각했는데
다른 점에 대해 좀 더 설명해주실수 있을까요 ?? 😃

eun-byeol · 2024-11-19T05:51:04Z

backend/src/test/java/com/ody/route/service/RouteClientCircuitBreakerTest.java

+        Boolean blocked = redisTemplate.hasKey(RouteClientKey.getBlockKey(routeClient));
+
+        assertAll(
+                () -> assertThat(failCount).isEqualTo(0),


요렇게도 가능합니다!

Suggested change

() -> assertThat(failCount).isEqualTo(0),

() -> assertThat(failCount).isZero(),

오호 isZero()라는 메서드도 있었네요 반영완료!

eun-byeol · 2024-11-19T05:58:21Z

backend/src/main/resources/common.yml

+route:
+  client:
+    cooldown:
+      duration: 3600000  # 1시간
+    failure:
+      max-count: 5       # 5회 연속 실패시 쿨다운
+      window: 300000    # 5분 내 실패 카운트


[질문]
사용되고 있는 부분이 없어 보여요. 이건 어디에서 사용하고 있나요?

프로퍼티로 관리하려했었는데 지금은 사용하고 있지 않아 제거했습니다!

hyeon0208 added 15 commits November 14, 2024 00:18

feat: Redis 설정 추가

0bf56ea

refactor: 서킷 브레이커 패턴을 적용해 RouteClient 관리

a5563c6

test: Redis 테스트 컨테이너 설정 추가

3993eef

refactor: 범위 내 비트 개수 반환 로직 수정

f26441f

test: CustomRedisTemplate 테스트 추가

a3a9a1a

refactor: 서킷브레이커 클래스명 수정

d264ff6

test: 레디스 테스트를 공통화한 추상 클래스 추가

5619da1

refactor: bit 자료형 대신 String 자료형으로 카운팅하도록 수정

f8e0a9f

test: RouteClientCircuitBreaker 테스트 추가

108b2ab

test: RouteClientManager 테스트 추가

2e356de

refactor: 로깅 레벨 수정

837bf4d

test: redisTemplate 초기화 로직 개행 추가

37f9eb8

chore: dev, prod elasticache 주소 추가

4edc449

refactor: 보안을 위한 ssl 연결 설정 추가

b82d4fc

refactor: 로컬을 고려한 ssl 연결 설정 제거

398963d

hyeon0208 added BE REFACTOR INFRA labels Nov 15, 2024

hyeon0208 requested review from eun-byeol, mzeong and coli-geonwoo November 15, 2024 11:09

hyeon0208 self-assigned this Nov 15, 2024

eun-byeol requested changes Nov 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refacator: API 의존성 관리 로직 개선 #910

refacator: API 의존성 관리 로직 개선 #910

hyeon0208 commented Nov 15, 2024 •

edited

Loading

eun-byeol left a comment

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

eun-byeol Nov 19, 2024

hyeon0208 Nov 19, 2024

		routeClientCircuitBreaker.recordFailCountInMinutes(routeClient);
		routeClientCircuitBreaker.determineBlock(routeClient);

		redis:
		host: ody-redis-001.ody-redis.vd1e5e.apn2.cache.amazonaws.com


		private static final String TEST_KEY = "test";

		@DisplayName("key의 counter를 증가시킨다.")

	() -> assertThat(failCount).isEqualTo(0),
	() -> assertThat(failCount).isZero(),

refacator: API 의존성 관리 로직 개선 #910

Are you sure you want to change the base?

refacator: API 의존성 관리 로직 개선 #910

Conversation

hyeon0208 commented Nov 15, 2024 • edited Loading

🚩 연관 이슈

📝 작업 내용

🏞️ 스크린샷 (선택)

🗣️ 리뷰 요구사항 (선택)

eun-byeol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyeon0208 commented Nov 15, 2024 •

edited

Loading