Skip to content

Conversation

@scars97
Copy link
Owner

@scars97 scars97 commented Jun 22, 2025

작업 이슈

AS-IS

  • 특정 기간동안 주문된 상품 데이터 조회
  • 애플리케이션 레벨에서 인기 상품 순위 정렬 작업 수행
override fun getPopularItemsOnTop10(period: Long): List<PopularItem> {
    // period 만큼 주문 상품 전체 조회

    // 중복 제거 후, 주문 수량 합 계산

    // 정렬된 주문 상품의 id로 실제 상품 조회

    // 인기 상품 변환
}

TO-BE

  • 스케줄러를 활용하여 특정 기간동안 주문된 상품 데이터 집계 후 저장
    • 스케줄러에서 인기 상품 순위 정렬 작업 수행
    • 레디스에 상품 ID 만 저장
  • 레디스에서 인기 상품 ID 조회 후 DB 조회
override fun getPopularItems(period: Long): List<PopularItem> {
    // 레디스에서 인기 상품 ID 조회

    // 인기 상품 ID로 실제 상품 조회

    // 인기 상품 변환
}

인기 상품을 조회할 때마다 수행되는 순위 정렬 작업이 스케줄러로 이동

  • CPU 작업을 줄일 수 있을 것으로 예상

@scars97 scars97 linked an issue Jun 22, 2025 that may be closed by this pull request
@scars97 scars97 changed the title [#8] 인기 상품 조회 방식 변경 [refactor] 인기 상품 조회 방식 변경 Jun 22, 2025
@scars97 scars97 merged commit c228ecf into master Jun 22, 2025
@scars97
Copy link
Owner Author

scars97 commented Nov 11, 2025

@claude pr 코드 리뷰를 부탁드립니다.

@claude
Copy link

claude bot commented Nov 11, 2025

Claude finished @scars97's task —— View job


인기 상품 조회 방식 리팩토링 코드 리뷰 ✅

전반적으로 성능 최적화를 위한 좋은 아키텍처 변경입니다. 실시간 계산 방식에서 스케줄링 기반의 사전 계산 방식으로 변경하여 API 응답 시간 개선과 CPU 부하 감소를 달성할 수 있을 것 같습니다.

주요 개선사항 ✨

1. 성능 최적화

  • 실시간 계산 제거: 매 요청마다 수행되던 집계 연산을 스케줄러로 이동
  • Redis 캐싱 활용: 사전 계산된 인기 상품 ID를 Redis에 저장하여 빠른 조회
  • API 응답 속도 개선: getPopularItems 메서드가 단순한 조회 로직으로 변경

2. 아키텍처 개선

  • 관심사 분리: 집계 로직을 별도 스케줄러로 분리하여 책임 명확화
  • 확장 가능한 구조: 여러 기간(3일, 7일)에 대한 집계를 쉽게 확장

코드별 세부 검토

ItemRankSchedule.kt

좋은 점:

  • 매일 자정 실행되는 cron 설정이 적절함
  • 여러 기간을 지원하는 PERIOD_LIST 구조
  • 집계 로직이 명확하게 구현됨

개선 제안:

// ItemRankSchedule.kt:16 - 하드코딩된 기간 값
val PERIOD_LIST = listOf(3L, 7L)

이 부분을 application.yml에서 설정할 수 있도록 @ConfigurationProperties@Value로 외부화하는 것을 고려해보세요.

ItemPersistenceAdapter.kt

좋은 점:

  • Redis 연동이 잘 구현됨
  • 에러 처리가 적절함

개선 제안:

// ItemPersistenceAdapter.kt:56-60 - JSON 파싱 방식
val trimmed = json.removePrefix("[").removeSuffix("]")
trimmed.split(",").map { it.trim().toLong() }

현재는 문자열 조작으로 JSON을 파싱하고 있는데, 다음과 같은 개선을 고려해보세요:

  1. JSON 라이브러리 사용: Jackson ObjectMapper나 Gson을 활용한 안전한 직렬화/역직렬화
  2. Redis 직렬화 개선: List를 직접 저장할 수 있는 Redis 직렬화 설정
// 개선안 예시
override fun getPopularItemIds(period: Long): List<Long> {
    val key = POPULAR_ITEMS.plus(period)
    return redisTemplate.opsForList().range(key, 0, -1)?.mapNotNull { 
        it.toLongOrNull() 
    } ?: emptyList()
}

ItemService.kt

좋은 점:

  • 로직이 매우 단순해져서 가독성이 향상됨
  • convertPopularItems 메서드가 순위 계산을 담당

개선 제안:

// ItemService.kt:30 - 개별 조회로 인한 N+1 문제
val items = itemIds.map { itemPort.getItems(it) }.toList()

현재 각 상품을 개별로 조회하고 있어 N+1 쿼리 문제가 발생할 수 있습니다. getItemsIn 메서드를 활용하여 벌크 조회로 개선하세요:

val items = itemPort.getItemsIn(itemIds)
// 순서 유지를 위한 정렬 필요
val sortedItems = itemIds.mapNotNull { id -> 
    items.find { it.id == id } 
}

테스트 코드 품질 👍

ItemRankScheduleTest.kt

  • 통합 테스트로 스케줄러 동작을 검증
  • 예상 순위 결과를 명확히 검증

ItemPersistenceAdapterTest.kt

  • Redis 연동 기능에 대한 Command/Query 분리 테스트
  • DynamicTest를 활용한 우아한 테스트 구조

잠재적 이슈 및 고려사항 ⚠️

1. 데이터 일관성

// ItemRankSchedule.kt:29 - 원자성 보장 필요
itemPort.updatePopularItemRank(it, sortedPopularItemIds)

스케줄러 실행 중 장애가 발생할 경우 일부 기간의 데이터만 업데이트될 수 있습니다. Redis Transaction이나 Lua Script를 활용한 원자적 업데이트를 고려하세요.

2. 에러 핸들링

  • Redis 연결 실패 시 fallback 전략 부재
  • 스케줄러 실행 실패 시 알림/로깅 메커니즘 필요

3. 성능 고려사항

  • 대용량 주문 데이터 처리 시 메모리 사용량 모니터링 필요
  • Redis 메모리 정책 및 TTL 설정 고려

권장사항 📋

  1. 설정 외부화: 하드코딩된 기간 값들을 설정 파일로 이동
  2. JSON 직렬화 개선: 안전한 JSON 라이브러리 활용
  3. 벌크 조회 적용: N+1 쿼리 문제 해결
  4. 에러 핸들링 강화: Redis 장애 시나리오 대응
  5. 모니터링 추가: 스케줄러 실행 상태 및 성능 메트릭 수집

결론 🎯

이번 리팩토링은 성능 최적화아키텍처 개선 측면에서 매우 의미있는 변경입니다. 실시간 계산으로 인한 CPU 부하를 스케줄러로 이동시키고, Redis를 활용한 캐싱으로 응답 속도를 크게 개선할 수 있을 것입니다.

제안한 개선사항들을 적용하면 더욱 안정적이고 확장 가능한 시스템이 될 것 같습니다. 특히 N+1 쿼리 문제와 JSON 직렬화 개선은 성능에 직접적인 영향을 줄 수 있어 우선 적용을 권장드립니다.


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