MV 기반 주간/월간 랭킹 배치 시스템 구축#422
Open
SukheeChoi wants to merge 36 commits into
Open
Conversation
- 배치 앱 2개(gddp 47 Job, mbod 43 Job) 구조/패턴 분석 - 통계 Tasklet SQL 패턴 4종 분류, UniqueRunIdIncrementer 동작 분석 - GoodsReviewTotal UPSERT vs DELETE+INSERT 트레이드오프 정리 - MV Score 계산 방식 A(메트릭 합산 후 score 1회 계산) 확정 - Redis(Speed Layer) vs MV(Batch Layer) 역할 분담 설계
- Job/Step 구성, Reader/Processor/Writer 패턴 10종 정리 - JdbcCursorItemReader, MyBatisCursorItemReader 설정 코드 - Multi-Step 조건 분기, CompositeItemWriter 체이닝 패턴 - 통계 SQL 패턴 (DELETE+INSERT, UPSERT), UPSERT 실물 XML - StepExecutionListener, SingleJobExecutionListener 전체 소스 - @StepScope + JobParameter 주입 패턴
- Composite VO Processor 패턴 (마일리지 소멸 Job) 추가 - ExecutionContext 기반 재시작/재개 패턴 (검색 인덱스 Job) 추가 - vs commerce-batch application.yml 설정 비교 분석 - MV 기반 주간/월간 랭킹 시스템 설계 문서 작성 - Score 방식 A 확정, DDL, Job 구조, API 확장, 구현 순서
- 시간 윈도우: 슬라이딩(매일 갱신) 확정, 캘린더 방식 기각 - Redis monthly 지수 감쇠(반감기 23일) vs MV 균등 합산 차이 분석 - period_key를 targetDate(20260416)로 변경 (ISO 주차/월 → 날짜) - 요구사항 R1~R4 대조 및 Phase별 충족 매핑 추가
- 균등 합산 vs 지수 감쇠 vs 일평균 비교 분석 및 판단 근거 - 전시 기간 편향 보정 방안 (일평균, 전환율) 검토 - 슬라이딩 vs 캘린더 윈도우 선택 근거 - 배치 앱 분석에서 배운 Tasklet vs Chunk 비율 - Lambda Architecture에서 Redis vs MV 공존 이유
- Score 전략: "구현 복잡도" 기준 제거, 비즈니스 의미 중심으로 재작성 - 균등 합산 채택 근거: 이커머스 업계 표준(총 실적 기준 베스트셀러) - 일평균/전환율 기각 근거: 공개 랭킹 보드 vs 내부 분석/추천 시스템의 목적 차이 - 슬라이딩 윈도우: 캘린더가 더 적합한 경우(정산/리포팅)도 함께 분석 - Chunk vs Tasklet: 비율의 이유와 현재 프로젝트에서 Chunk를 쓰는 근거 - Lambda Architecture: 운영 관점(장애 내성, 데이터 검증, CS 대응) 추가
- 3가지 방안 비교: Java 전체 처리 vs 전체 INSERT 후 삭제 vs SQL 완료 - SQL 실행 순서(GROUP BY→SELECT→ORDER BY→LIMIT)로 TOP 100 보장 분석 - 배치 앱 12개 매퍼에서 윈도우 함수(RANK, ROW_NUMBER 등)로 TOP-N 처리하는 패턴 확인 - Score 공식 이중 관리 트레이드오프 분석
- Tasklet(INSERT INTO...SELECT) 장단점 상세 분석 - GoodsBestMapper와 동일한 Tasklet 구현 예시 SQL - Chunk가 Tasklet보다 유리해지는 전환점 4가지 정리 - 현재 설계에서 Chunk를 쓰되 Reader SQL로 비효율 최소화하는 전략
- 사전 집계는 입력 볼륨, Chunk는 출력 볼륨 — 서로 다른 문제 - 대규모 이커머스에서도 이 작업에는 Tasklet이 효율적인 이유 - Chunk가 진짜 필요한 시나리오 (등급 산정, 마일리지, 검색 인덱스) - Chunk를 요구한 의도와 시니어 관점의 답변 전략
- Chunk가 보편적 선택인 이유: retry, skip, restart, 모니터링 등 프레임워크 운영 기능 - Tasklet이 유리한 특정 조건 3가지 (SQL 완결, retry 불필요, 부분 완료 무의미) - 배치 앱 Tasklet 비율을 일반화하지 않도록 수정 - 설계 문서: Reader SQL에 score+LIMIT 100 반영, faultTolerant+retry 활용 명시
- Tasklet(SQL 중심) vs Chunk(운영 기능 활용) 코드 레벨 비교 예시 추가 - 배치 90개 Job에서 retry/skip/restart 미사용 사실 기록 - 미사용이 운영 리스크이며, 우리 설계에서 보완하는 것의 의미 분석
- Chunk Best Practice를 이론/우리 설계/배치 프로젝트 3관점으로 비교 - ExponentialBackOffPolicy, allowStartIfComplete 설계 반영 - Cursor Reader 멀티스레드 제약 명시 - Writer skip 시 chunk scan 문제 분석 - 배치 90개 Job의 retry/skip 미사용에 대한 해석
- GROUP BY 집계 쿼리에서 PagingReader의 치명적 문제 (페이지마다 집계 재실행) - CursorReader 멀티스레드 불가 원인 (ResultSet 공유 상태) - 커넥션 점유 해법: Replica DataSource 분리 (배치 프로젝트 패턴) - 병렬화 시 전환 경로: PagingReader가 아닌 Partitioning - 설계 문서 Best Practice 테이블에 Cursor 선택 근거 반영
- Job 구조를 3-Step(cleanup → partitioned aggregate → merge)으로 변경 - Partitioner: product_id 범위 분할, Worker별 독립 CursorReader - 스테이징 테이블 도입: 병렬 집계 결과 수집 → mergeStep에서 Global TOP 100 - 성능 산정: 상품 100만 기준 단일 30초 → Partitioning 12초 (3배) - 블로그 소재: 고민 흐름(Chunk→Cursor→멀티스레드 한계→Partitioning) 기록
- Partitioning 도입 후 멱등성: cleanup에서 스테이징도 함께 DELETE → 전체 재실행이 안전 - RunIdIncrementer 결정: 파라미터 보존 + run.id 증가로 재실행 허용 - 설계 결정 요약 테이블 갱신: Partitioning, 스테이징, RunIdIncrementer 반영 - 블로그 소재 10, 11번 추가
- 소재 1~11 모두 대화 흐름에서 어떤 질문/반론이 고민을 촉발했는지 기록 - 소재 1: 전시 기간 편향 보정 논의에서 시작 - 소재 2: carry-over vs 캘린더 기간 질문에서 시작 - 소재 3: "Chunk가 보편적이라는데?" 반론으로 시각 교정 - 소재 4: "Redis에 이미 있는데 왜 MV를 만드는가?" - 소재 5: "전부 INSERT하고 삭제하는 게 비효율 아닌가?" - 소재 6: "사전 집계가 있어야 Chunk가 유용한 건가?" - 소재 7: Best Practice 문서 받고 3관점 교차 분석 - 소재 8: "CursorReader를 선택한 이유가 뭐야?"
- weekly/monthly를 MV 단일 소스로 변경 (Redis fallback 제거) - 다른 공식(감쇠 vs 균등)의 결과를 fallback으로 쓰면 데이터 불일치 - API 구조: daily→Redis, weekly/monthly→MV (fallback 없이 빈 결과 반환) - 블로그 소재 4에 단일 소스 원칙 판단 과정 기록
- 전체 재계산 유지 근거: Late-Arriving Fact(지연 취소)로 과거 데이터 변경 발생
- 증분 계산의 전제("과거 불변")가 이커머스에서 깨지는 이유 분석
- 전일 MV fallback: 같은 공식의 1일 stale 결과 (데이터 불일치 아닌 시간 지연)
- 데이터 보존 정책: 3일분 보존, 이전 데이터 정리
- Redis weekly/monthly: 검증 후 제거, daily carry-over만 유지
- ProductRankingMvJobConfig: cleanup → partitioned aggregate → merge - CleanupTasklet: 당일 MV/staging DELETE + 3일 이전 데이터 정리 - Partitioner: product_id 범위 분할 (gridSize=4) - Worker: JdbcCursorItemReader(GROUP BY + LOG10 score) → staging INSERT - mergeStep: ROW_NUMBER() OVER → Global TOP 100 → MV INSERT - faultTolerant + retry(3) + ExponentialBackOffPolicy - DDL: mv_product_rank_weekly/monthly + staging 테이블 - 설계 문서 Phase 업데이트
- MvProductRank 엔티티 (MappedSuperclass + Weekly/Monthly 분리) - MvProductRankRepository 인터페이스 + JPA 구현체 - RankingFacade 수정: daily→Redis, weekly/monthly→MV 단일 소스 - 전일 MV fallback: 당일 데이터 없으면 전일 period_key로 조회 - Redis weekly/monthly prefix 제거 (daily만 유지)
- 주간 Job 정상 실행: 150개 상품 시드 → TOP 100 적재 검증 - 월간 Job 정상 실행: 30일 데이터 집계 검증 - 멱등성: 같은 파라미터 2회 실행 → 데이터 2배 아닌 동일 - 엣지케이스: 데이터 없음, 7일 미만, 100개 미만 상품 - 취소 반영: cancel_amount가 score에 반영되어 순위 변동
- Phase 2, 3 상태 ✅로 업데이트 (구현 완료, 테스트 코드 작성) - 테스트 실행 가이드: 사전 조건, 명령어, 실패 시 확인사항 - 시나리오 검증 절차: 인프라 기동 → 시드 → 배치 실행 → API 검증 → 멱등성/fallback 확인 - PR 리뷰 포인트 후보 3개 - 블로그 구조 가이드: 소재 문서 → 블로그 섹션 매핑
- DISTINCT product_id를 사전 조회하여 행 수 기준으로 파티션 경계 결정 - product_id에 gap이 있어도 파티션 간 처리량이 균등하게 분배 - 기존 MIN/MAX ID 범위 균등 분할은 gap 비율에 따라 불균형 발생
- PR Summary에서 "실환경 검증" 표현 제거 - 리뷰 포인트에서 참고자료/사례를 PR 본문으로 이동 - 블로그 소재 문서에 참고자료 4건 + 참고사례 5건 + 작성 참고 2건 정리 - 10만 건 대규모 테스트 프롬프트 작성 (다른 환경에서 실행용)
- 10-technical-writing-plan.md: 과제 가이드 기준 방향 조정, 글 구조, 소재 배치 - velog-techwriting-vol10.md: 일간/주간/월간 랭킹 배치 설계기 초안
- 100,000 상품 + 3,000,000 메트릭 행 벌크 시드 (batchUpdate 1,000건 단위) - 6가지 트렌드 패턴 (급상승/장기강자/하락/바이럴/취소높음/일반) - Weekly 2,205ms, Monthly 2,564ms — 4 Partition 균등 분배 확인 - Weekly 1위=급상승(p=5000), Monthly 1위=장기강자(p=15000) 시간 윈도우 검증 - Testcontainers innodb-buffer-pool-size=256M, Gradle -Xmx2g 설정 - BeforeEach DELETE→TRUNCATE 전환 (대규모 테스트 후 정리 성능)
- gddp: 47→49 Job (SapItemSync 추가), Stub 5개 식별 - mbod: 43→48 Job, 통계 11개, memberSyncJob(3 Step) 추가 - UniqueRunIdIncrementer: addLong(timestamp) → addString(UUID+timestamp) - 비교 테이블/부록 수치 갱신, MV 단일 소스 원칙 반영 - 캡처 파일 경로 수정
- new JdbcTemplate(dataSource) → 필드 주입 jdbcTemplate으로 통일 - Javadoc을 기존 RankingCorrectionJobConfig 수준으로 간소화
- gridSize=1 vs gridSize=4 성능 비교 테스트 프롬프트 작성 - PR 테스트 시나리오 8개로 정리 (시각화를 대규모 테스트에 통합) - 성능 예시를 실제 측정값(10만 상품 기준)으로 교체
📝 WalkthroughWalkthrough주간/월간 랭킹 조회를 Redis 기반에서 MV(Materialized View) 기반 저장소로 전환하고, RankingFacade의 라우팅을 조정하여 일별 조회는 Redis로, 주간/월간 조회는 MV로 처리하도록 변경했다. 관련 JPA 엔티티, 리포지토리, 배치 작업 및 엔드-투-엔드 테스트를 추가했다. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Facade as RankingFacade
participant MVRepo as MvProductRankRepository
participant DB as Database (MV tables)
participant Hydrator as ProductWithBrand Hydrator
Client->>Facade: getRankings(scope="weekly", date, page, size)
Facade->>Facade: scope 검사 (weekly/monthly -> MV)
Facade->>MVRepo: findByPeriodKeyAndScope(periodKey, "weekly", pageable)
MVRepo->>DB: 쿼리 mv_product_rank_weekly where period_key
DB-->>MVRepo: 결과 (rows)
alt 결과 없음
MVRepo->>DB: 쿼리 전날 period_key
DB-->>MVRepo: 결과 (rows or empty)
end
MVRepo->>MVRepo: totalElements clamp 및 totalPages 계산
MVRepo->>Hydrator: productId 목록으로 상품+브랜드 조회 요청
Hydrator->>DB: product / brand 조회
DB-->>Hydrator: productWithBrand 데이터
Hydrator-->>MVRepo: hydrated rows
MVRepo-->>Facade: PagedRankingResponse
Facade-->>Client: 응답 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (14)
build.gradle.kts (1)
85-85: 모든 서브프로젝트에 일괄-Xmx2g적용에 대한 재검토가 필요하다.대규모 배치 E2E 테스트(100k × 30d)에 필요한 변경인 점은 이해되나,
subprojects블록 전역으로 적용되어 해당 E2E가 없는 경량 모듈 테스트까지 모두 2G 힙을 점유한다.maxParallelForks = 1이라 단일 프로세스이지만, CI 내 다른 병렬 Job과 결합되면 메모리 경합이 커질 수 있다.
- 왜 문제인가: 실제 고부하 테스트는
apps/commerce-batch에 국한된 반면, 전 모듈 테스트 JVM에 큰 힙을 할당하면 로컬/CI 자원 낭비와 GC 압박 테스트가 이루어지지 않는다.- 수정안: 해당 모듈(
project(":apps:commerce-batch")) 또는ProductRankingMvJobE2ETest가 포함된 프로젝트에만-Xmx2g를 부여하고, 나머지는 기존 기본값을 유지한다. 필요 시 해당 모듈build.gradle.kts에서tasks.test { jvmArgs(...) }로 오버라이드한다.- 추가 테스트: CI에서 모듈별 peak RSS 측정을 한 번 수행해, 상향 범위가 적절한지 회귀적으로 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` at line 85, The global jvmArgs("-Xshare:off", "-Xmx2g") in the subprojects block should be removed and the 2G heap setting applied only to the module that needs it (project(":apps:commerce-batch")) or to the specific test task containing ProductRankingMvJobE2ETest; update build.gradle.kts to stop setting -Xmx2g in subprojects and instead add a tasks.test { jvmArgs("-Xshare:off","-Xmx2g") } (or equivalent test task configuration) inside the commerce-batch project (or the project that contains ProductRankingMvJobE2ETest) so other modules keep the default heap and only the heavy E2E test uses 2G.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlySpringDataRepository.java (1)
11-13: 페이징과 인덱스 정합은 확인되었으나, 정렬 보조키 누락이 잠재적 위험이다.스키마에
INDEX idx_period_ranking (period_key, ranking)이 존재하므로 주 경로는 인덱스 커버된다. 다만ranking단독 정렬은 동률(ranking중복)이 발생할 경우 페이지 간 순서가 불안정해질 수 있다.
- 왜 문제인가: 배치가
ROW_NUMBER() OVER로 생성하므로 현재는ranking유니크가 보장되지만, 향후 TIE 허용이나 부분 갱신으로 중복이 생기면 페이지 경계에서 레코드가 누락/중복 노출될 수 있다.- 수정안:
findByPeriodKeyOrderByRankingAscIdAsc(...)처럼 타이브레이커 보조키(id혹은productId)를 추가한다. 인덱스도 필요 시(period_key, ranking, id)로 확장한다.- 추가 테스트: 동일
ranking값이 2건 이상 포함된 고정 데이터로 페이지 사이즈 경계에서 누락/중복이 없는지 검증한다.As per coding guidelines: "페이징 시 fetch join 위험 등을 점검한다. 쿼리 조건 누락/과다 조회, 정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlySpringDataRepository.java` around lines 11 - 13, 현재 findByPeriodKeyOrderByRankingAsc(String periodKey, Pageable pageable)는 ranking 동률 발생 시 페이지 정합성이 깨질 위험이 있으니 페이징 tiebreaker를 추가해 안정화하세요: repository에서 메서드명을 findByPeriodKeyOrderByRankingAscIdAsc(...)로 변경해 ranking에 이어 id(또는 productId)를 오름차순으로 추가 정렬하고, 필요하면 DB 인덱스도 (period_key, ranking, id)로 확장하세요; 또한 페이징 경계 케이스를 검사하는 테스트(동일 ranking 값이 2건 이상인 고정 데이터로 페이지 사이즈 경계에서 누락/중복 없음)를 추가해 검증하세요.apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.java (1)
13-17: 도메인 엔티티의equals/hashCode정책 부재다.
@MappedSuperclass의 하위 엔티티들이 영속/비영속 상태를 오가며 비교될 때Object기본 동등성에 의존하면 캐시/세트/비교 검증에서 혼란이 발생할 수 있다. MV는 조회 전용이라 당장 이슈가 되진 않으나, 도메인 엔티티 일관성을 위해 식별자 기반equals/hashCode를 정의해두는 편이 안전하다.
- 왜 문제인가: 향후 MV 결과를
Set/Map키로 쓰거나 JPA 2차 캐시/병합 경로가 추가되면 예기치 못한 중복/불일치가 발생할 수 있다.- 수정안:
id기반equals/hashCode를MvProductRank에 정의한다(Lombok@EqualsAndHashCode(of = "id")또는 수동 구현). null id 비교 정책도 함께 명시한다.As per coding guidelines: "equals/hashCode는 식별자 기반 또는 값 기반으로 일관되게 설계한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.java` around lines 13 - 17, Add a consistent identifier-based equality policy to MvProductRank by implementing equals and hashCode using the private Long id field: locate the MvProductRank class and either annotate it with Lombok `@EqualsAndHashCode`(of = "id") or add manual implementations that return true only when both objects are the same class and non-null ids are equal, and treat entities with null id as not equal (and compute hashCode accordingly). Ensure the implementations are placed on the MvProductRank abstract class so all subclasses inherit the id-based equality semantics.apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.java (1)
7-12:scope를String으로 노출한 점이 아쉽다.도메인 계층 리포지토리 시그니처가
String scope로 열려 있어 호출 측에서 오탈자("weekley", 대소문자 등)가 유입될 수 있으며, 구현체(MvProductRankJpaRepository)의switch에서IllegalArgumentException으로 런타임에만 실패가 드러난다.
- 왜 문제인가: 운영 관점에서 라우팅 오류가 정적으로 잡히지 않고 요청 단위 예외로 전파되며, 장애 원인 분석 시 원인이 호출 문자열에 흩어져 있어 추적이 늦어진다.
- 수정안:
RankingScopeenum(WEEKLY,MONTHLY)을 도입해 시그니처를findByPeriodKeyAndScope(String periodKey, RankingScope scope, Pageable)로 바꾼다. 외부(API)에서 들어오는 문자열은 경계에서 enum으로 변환(RankingKeyResolver등)한다.- 추가 테스트: 잘못된 scope 문자열이 들어올 때 경계(파서)에서
CoreException으로 변환되는지 검증하는 테스트를 추가한다.As per coding guidelines: "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다. 도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.java` around lines 7 - 12, 현재 MvProductRankRepository의 메서드 시그니처 findByPeriodKeyAndScope(String periodKey, String scope, Pageable pageable)가 문자열로 scope를 노출해 호출측 오타/대소문자 문제를 유발하므로, 도메인 안전성을 위해 scope 타입을 String에서 새로운 enum RankingScope(WEEKLY, MONTHLY 등)으로 바꿔 시그니처를 findByPeriodKeyAndScope(String periodKey, RankingScope scope, Pageable pageable)로 변경하고, MvProductRankJpaRepository의 switch/처리 로직을 RankingScope 기반으로 리팩토링하며 외부에서 들어오는 원시 문자열은 경계에 위치한 파서(예: RankingKeyResolver)에서 변환하고 잘못된 값은 CoreException으로 변환해 처리하도록 변경 및 해당 변환 실패 케이스를 검증하는 테스트를 추가하세요.apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java (4)
111-113: weekly=6일/monthly=29일 계산이 두 곳에 중복되어 있다.
createPartitioner의int days = "weekly".equals(scope) ? 6 : 29;와stagingReader의 동일 식이 중복되어 있어, 기간 정책이 바뀔 때 한 쪽을 빠뜨리면 Partitioner와 Reader의 기간 윈도가 어긋나 파티션 범위와 실제 집계 범위가 달라진다. 상수 또는 헬퍼 메서드로 단일화하는 것을 권한다.♻️ 제안 변경
+ private static int windowDays(String scope) { + return "weekly".equals(scope) ? 6 : 29; + }그리고 양쪽에서
windowDays(scope)를 호출한다.Also applies to: 180-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java` around lines 111 - 113, The days calculation for scope is duplicated between createPartitioner and stagingReader; extract the logic into a single helper (e.g., a private static method windowDays(String scope) or a constant lookup) and replace both occurrences (the ternary int days = "weekly".equals(scope) ? 6 : 29;) with calls to windowDays(scope) so both createPartitioner and stagingReader use the same source of truth for the partition/aggregation window.
259-287:mergeStep의 DML에 테이블명이 문자열 포매팅으로 주입된다.
scope는 switch로 화이트리스트 검증을 통과한 값이므로 SQL injection 위험은 실질적으로 없다. 다만String.format기반 DDL/DML 조립은 장기적으로 인자가 늘어날 때 취약점 경로가 되기 쉬우므로, 테이블명 상수를 모듈 레벨Map<String, String>또는 enum(ScopeType.WEEKLY.mvTable())으로 단일화하고,CleanupTasklet#resolveMvTable과 이 곳의 switch를 같은 소스에서 재사용하는 것을 권한다. 또한 현재 구조에서mergeStep의 tasklet이@Bean/@JobScope로 등록되어 있는 반면CleanupTasklet은 별도 컴포넌트로 분리되어 있어 일관성도 떨어진다.추가로,
INSERT ... SELECT ... LIMIT 100에서period_key가 staging에 여러 개 존재할 수 있음을 가정하지만(retention purge 이전의 이전 기간 데이터), 실제로WHERE period_key = ?로 필터되므로 기능상 안전하다. 다만ORDER BY score DESC동점 시 어떤 100건이 선택될지 결정적이지 않은 점은 앞서 지적한 tie-breaker 이슈와 이어진다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java` around lines 259 - 287, The tasklet for mergeStep injects a table name via String.format/switch which is fragile and duplicated with CleanupTasklet; replace the ad-hoc switch in mergeStep with a single source of truth (e.g., a module-level Map<String,String> or an enum like ScopeType with a mvTable() method) and call that resolver (reuse CleanupTasklet#resolveMvTable or move it into a shared helper) instead of inline formatting, make the tasklet registration consistent with CleanupTasklet (both as `@Bean/`@JobScope or both as components), and add a deterministic tiebreaker to the INSERT ... SELECT ... ORDER BY score DESC (e.g., ORDER BY score DESC, product_id) to ensure stable LIMIT 100 results.
109-150: Partitioner가 전체 product_id 리스트를 드라이버 메모리로 올리는 구조이다.현재 구현은
SELECT DISTINCT product_id FROM product_metrics WHERE metric_date BETWEEN ? AND ? ORDER BY product_id의 결과 전량을List<Long>으로 로딩한 뒤, 각 파티션의 min/max를 산출한다. 10만 건 기준으로는 수백 KB~수 MB이므로 현 PR의 성능 테스트는 통과하지만, 상품 카탈로그가 수백만 규모로 커지면 드라이버 힙 압박 및 배치 시작 지연이 커진다. DB 단에서 윈도우 함수(NTILE)나percentile기반 경계를 계산해 min/max 페어만 반환하도록 하면 메모리 의존을 제거할 수 있다.또한
product_metrics의idx_metric_date만으로는BETWEEN + DISTINCT product_id ORDER BY product_id가 비효율적일 수 있다.(metric_date, product_id)복합 인덱스 추가로 인덱스-only 스캔이 가능해진다(테스트 DDL은 현 PR 범위이므로 운영 DDL에도 반영 필요).
109-128: 데이터 없는 날짜에도 빈 파티션을 1개 반환하여 workerStep이 0,0 범위로 동작한다.
productIds.isEmpty()일 때partition01개만 반환하고minProductId=0, maxProductId=0을 주입한다.stagingReader의 쿼리는pm.product_id BETWEEN 0 AND 0이 되어 결과 0건을 돌려주므로 기능적으로는 문제없이 Job이 COMPLETED로 끝나지만,gridSize=4가 아닌 1개 파티션만 생성되는 점이 의도와 부합하는지 확인이 필요하다. 더 명확하게는return Map.of();(빈 Map)을 반환하거나, Partitioner에서 throw하지 않고 상위 플로우에서 "데이터 없음" 브랜치로 분기시키는 것이 운영 관찰성 관점에서 낫다.(테스트
noDataProducesEmptyMv가 COMPLETED를 기대하고 있으므로 동작은 유지하되, 로깅/분기를 명시적으로 개선하는 선택지를 권한다.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java` around lines 109 - 128, The createPartitioner method currently returns a single dummy partition ("partition0" with minProductId=0 and maxProductId=0) when productIds.isEmpty(), which creates only one worker instead of honoring gridSize; change the empty-data branch in createPartitioner to return an empty Map (e.g., Collections.emptyMap() or Map.of()) and update the log to be explicit (e.g., log.warn("[Partitioner] no data for date range {} ~ {}, returning 0 partitions", startDate, endDate)); this preserves the job COMPLETED behavior in tests while making partitioning and observability correct and allows upstream flow/step logic to detect “no partitions” and handle the no-data case.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
76-77:size == 0방어가 없어 ArithmeticException 가능성이 있다.
totalPages = (int) Math.ceil((double) totalElements / size)에서size == 0이면 정수 나눗셈이 아니지만,PageRequest.of(page, 0)자체가 먼저IllegalArgumentException을 던진다. 다만 외부 계층에서 size 검증이 누락되면PageRequest가 먼저 터지므로 일관된 오류 포맷으로 노출되지 않는다. Facade 초입에서page >= 0,size > 0을CoreException(ErrorType.BAD_REQUEST)로 통일해 노출하는 것을 권한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java` around lines 76 - 77, Add defensive validation at the start of the RankingFacade method that computes pagination: check that page >= 0 and size > 0 and if not throw new CoreException(ErrorType.BAD_REQUEST). Ensure this validation runs before calling PageRequest.of(page, size) and before computing totalPages (the lines with totalElements = Math.min(totalElements, MAX_RANKING_SIZE) and totalPages = (int) Math.ceil((double) totalElements / size)) so you avoid ArithmeticException or inconsistent errors from PageRequest.apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java (3)
313-326: "데이터 없는 날짜 = 빈 MV" 테스트가COMPLETED가정만 검증한다.현재는
BatchStatus.COMPLETED와mvCount == 0만 확인하지만, 앞서 지적한 Partitioner의 "빈 파티션 0,0" 처리 경로를 함께 검증하려면 staging 행 수(SELECT COUNT(*) FROM mv_product_rank_staging WHERE period_key = ?)와 이전 기간(period_keyretention 경계)에 대한 잔존 여부도 어서션에 포함할 것을 권한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java` around lines 313 - 326, The test noDataProducesEmptyMv currently only asserts BatchStatus.COMPLETED and that mv_product_rank_weekly has zero rows for TARGET_DATE; extend it to also assert that mv_product_rank_staging has zero rows for the same period_key and verify that rows from previous periods outside the retention boundary are not present (i.e., query mv_product_rank_weekly for the previous period_key/retention boundary and assert count is zero); update the assertions after calling runJob("weekly") to include these two extra checks so the Partitioner empty-partition ("0,0") and retention-cleanup paths are validated.
57-64:seedProducts(단건 루프)의 테스트 실행 시간이 길다.본 메서드는
weeklySuccess,monthlySuccess,idempotentDoubleExecution등 대부분의 테스트가 사용한다. 1건씩 INSERT를 반복하므로 150건×수 테스트만 해도 수백 회 JDBC round-trip이 발생한다. 이미seedProductsBulk가 존재하므로 일반 테스트도batchUpdate기반으로 통일하면 총 테스트 시간이 짧아지고 트랜잭션/로깅 노이즈도 줄어든다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java` around lines 57 - 64, The current seedProducts method performs single-row JDBC updates in a loop causing many round-trips; change it to use a batch insert (e.g., call or mirror seedProductsBulk) so tests like weeklySuccess, monthlySuccess, and idempotentDoubleExecution use jdbcTemplate.batchUpdate (or reuse seedProductsBulk) to insert all products in one JDBC call, preserving the same columns/values and parameter order as the existing per-row SQL; update the seedProducts method body to build the batch params and call batchUpdate to reduce round-trips and test runtime.
344-557:printRankingResults는 어서션이 없어 회귀 탐지가 불가능하다.콘솔 출력 중심의 시각화 테스트로 보인다. 유지 목적이 명확하다면
@Tag("visualization")또는@Disabled를 기본값으로 두고, 회귀 감시용으로는 유형별 대표 product의 순위(예: 급상승 카테고리는 일간보다 주간 순위가 높아야 함 등)에 대한 최소 어서션을 추가하는 것을 권한다. 현재 구조에서는 스코어 공식이 바뀌어 완전히 다른 결과가 나와도 테스트는 통과한다.또한
LEFT JOIN mv_product_rank_weekly ... ON d.product_id = w.product_id AND w.period_key = ?구문은w가 MV에 있어도ranking정렬이 daily 기준이라, 출력 포맷이 혼란스러울 수 있다. 시각화 목적이면 그대로 두어도 되지만, 어서션 추가 시에는ORDER BY w.ranking을 별도로 뽑아 비교해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java` around lines 344 - 557, This test (printRankingResults) has no assertions and should be marked as a visualization-only test and given a few smoke assertions to detect regressions: add `@Tag`("visualization") (or `@Disabled`) to the printRankingResults method and after populating data and running runJob("weekly")/runJob("monthly") extract the compareRows results to assert representative product rank relationships (e.g., for product ids 1-3 assert daily_rank < weekly_rank (recent spike => better daily), for ids 17-20 assert monthly_rank <= weekly_rank <= daily_rank (long-term strong), for ids 14-15 assert weekly_rank < daily_rank (recent decline), and for id 8 assert daily_rank << weekly_rank/monthly_rank (today viral)). When asserting values, use the rows returned from compareRows and the mv_product_rank_weekly / mv_product_rank_monthly entries (w.ranking / mo.ranking) rather than relying on d.ranking ordering if you need MV-specific ordering.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankJpaRepository.java (1)
17-35:scope기반 분기의 중복을 줄이고 검증 실패 경로를 통일해야 한다.
findByPeriodKeyAndScope/countByPeriodKeyAndScope모두 동일한 weekly/monthly switch를 반복 구현하고 있어 향후quarterly등 스코프 추가 시 두 곳을 빠짐없이 손봐야 한다. 또한IllegalArgumentException이 Facade 상위에서CoreException으로 변환되지 않으면 API 응답 포맷이 깨지므로, 무효 scope는 Facade 진입 시점에서CoreException(ErrorType.BAD_REQUEST)으로 처리하거나 본 리포지토리 내부에서ScopeTypeenum으로 제한하는 편이 안전하다. 현재는 Facade 쪽에서"weekly","monthly"만 전달하지만, 외부에서 임의 문자열이 주입되면 500으로 떨어진다.추가 테스트로 "unsupported scope 문자열 입력 시 API 계층에서 4xx 응답이 나오는지"를 검증하는 테스트를 제안한다.
♻️ 제안 변경(공통화 예시)
- `@Override` - public List<MvProductRank> findByPeriodKeyAndScope(String periodKey, String scope, Pageable pageable) { - return switch (scope) { - case "weekly" -> weeklyRepository.findByPeriodKeyOrderByRankingAsc(periodKey, pageable) - .stream().map(r -> (MvProductRank) r).toList(); - case "monthly" -> monthlyRepository.findByPeriodKeyOrderByRankingAsc(periodKey, pageable) - .stream().map(r -> (MvProductRank) r).toList(); - default -> throw new IllegalArgumentException("Invalid scope: " + scope); - }; - } - - `@Override` - public long countByPeriodKeyAndScope(String periodKey, String scope) { - return switch (scope) { - case "weekly" -> weeklyRepository.countByPeriodKey(periodKey); - case "monthly" -> monthlyRepository.countByPeriodKey(periodKey); - default -> throw new IllegalArgumentException("Invalid scope: " + scope); - }; - } + `@Override` + public List<MvProductRank> findByPeriodKeyAndScope(String periodKey, String scope, Pageable pageable) { + return switch (scope) { + case "weekly" -> List.copyOf(weeklyRepository.findByPeriodKeyOrderByRankingAsc(periodKey, pageable)); + case "monthly" -> List.copyOf(monthlyRepository.findByPeriodKeyOrderByRankingAsc(periodKey, pageable)); + default -> throw new IllegalArgumentException("Invalid scope: " + scope); + }; + }(불필요한
.stream().map(r -> (MvProductRank) r).toList()캐스트는 제네릭 상한으로 이미 호환되므로 생략 가능하다.List<MvProductRankWeekly>는List<? extends MvProductRank>이다.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankJpaRepository.java` around lines 17 - 35, Consolidate the duplicate weekly/monthly branching in MvProductRankJpaRepository by validating and normalizing scope into a single enum (e.g., ScopeType with WEEKLY/MONTHLY) at repository entry (or have the Facade throw CoreException(ErrorType.BAD_REQUEST) on invalid scope) and then switch once to select the correct repository (weeklyRepository or monthlyRepository) for both findByPeriodKeyAndScope and countByPeriodKeyAndScope; remove the unnecessary .stream().map(...) casts when returning List<MvProductRank> and ensure invalid scope paths raise a controlled CoreException or accept only ScopeType to guarantee 4xx behavior for unsupported scope strings.apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/step/CleanupTasklet.java (1)
34-59: Retention 삭제를 청크 분할 혹은 별도 트랜잭션으로 분리할 것을 검토해야 한다.현재
execute는 1개 트랜잭션 내에서 (a) targetDate 정확 매칭 DELETE × 2, (b) cutoff 이전 DELETE × 2를 순차 실행한다. 정상 운영에서는 3일 보존 정책 덕에 삭제 대상이 크지 않겠지만, 배치가 수일간 지연되었다가 재개되는 상황(장애/주말 누락 등)에서는 cutoff DELETE가 수십만~수백만 행에 도달할 수 있으며, 이는 다음 문제를 유발한다.
- InnoDB 언두 로그/락 영역이 MV/staging 모두에서 장시간 유지되어 온라인 조회(
RankingFacade.getFromMv)와 경합.- 동일 트랜잭션 내에서 롤백이 발생하면 "정확 매칭 DELETE"까지 함께 되돌아가
allowStartIfComplete(true)의 재실행 시 상태가 애매해짐.운영 관점에서는 retention purge를 별도 Tasklet/스텝으로 분리하거나,
DELETE ... LIMIT N루프로 청크 삭제 후 커밋하는 패턴이 안전하다. 추가로 "period_key < cutoffKey"는 staging 테이블에서 인덱스가 없어 풀스캔을 수행하는 점(앞선 스키마 코멘트 참고)과도 맞물리므로 같이 개선 권한다.추가 테스트로 "3일 이상 밀린 상태에서 실행되어 구버전 다수가 존재하는 경우" 시나리오를 두고, 삭제 건수 로그 및 소요 시간을 점검하는 것을 권한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/step/CleanupTasklet.java` around lines 34 - 59, The retention purge ("period_key < cutoffKey") in CleanupTasklet.execute should be separated from the exact-match deletes to avoid long-running single transactions: either move the cutoff deletes into a dedicated Step/Tasklet (e.g., new RetentionPurgeTasklet) and call it after the exact-match step, or implement chunked deletes inside CleanupTasklet.execute by looping deletes with "DELETE FROM <table> WHERE period_key < ? LIMIT N" for both resolveMvTable(scope) and mv_product_rank_staging, committing between chunks; reference the symbols CleanupTasklet.execute, resolveMvTable(scope), mv_product_rank_staging, RETENTION_DAYS, DATE_FORMATTER and ensure logs still report purged counts and timings per chunk or in the new Tasklet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 43-50: getRankings currently does a switch(scope) which throws NPE
when scope is null; guard against null before the switch in
RankingFacade.getRankings by resolving scope to a safe value or validating it
(e.g., if scope == null -> use a default like "default" or call the same
fallback path as the switch default) and then perform the switch, or explicitly
throw a clear IllegalArgumentException; alternatively consider introducing a
ScopeType enum and require/convert scope to that enum before branching so the
null/invalid case is handled consistently with memberId/date checks and routed
to getFromRedis/getFromMv as appropriate.
- Around line 52-74: The fallback currently uses mvResults.isEmpty() which
conflates "no data for period" with "page out of range" and can wrongly replace
today's data with yesterday's; in getFromMv, first call
mvProductRankRepository.countByPeriodKeyAndScope(date, scope) to get
totalElements and only if totalElements == 0 perform the previousDate fallback
(recompute previousDate, set totalElements =
countByPeriodKeyAndScope(previousDate, scope) and log), then call
mvProductRankRepository.findByPeriodKeyAndScope(...) to load mvResults using
PageRequest.of(page, size); ensure you reference and update totalElements and
mvResults only when count indicates zero so that page-out-of-range cases return
empty results for the existing period instead of falling back.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.java`:
- Around line 43-44: MvProductRank currently declares private LocalDateTime
createdAt with `@Column`(nullable = false) but has no JPA-side auto-population, so
saving via JpaRepository will set NULL and violate the constraint; update the
entity to auto-populate createdAt (either add a `@PrePersist` method in
MvProductRank that sets createdAt = LocalDateTime.now() if null, or annotate the
field with Hibernate's `@CreationTimestamp`) and add a repository unit test that
saves MvProductRank via save() to assert createdAt is non-null after persist.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java`:
- Around line 75-83: The job flow currently ends with
.start(cleanupStep).on("FAILED").end(), which marks the Job as COMPLETED when
cleanupStep fails; update the transition to use .fail() instead of .end() so a
cleanupStep failure results in a FAILED JobStatus (modify the JobBuilder flow in
ProductRankingMvJobConfig to replace .end() with .fail() for the "FAILED" branch
of cleanupStep). Additionally add one E2E test that injects a CleanupTasklet
that throws, runs the job via jobLauncherTestUtils.launchJob(...), and asserts
the returned BatchStatus is FAILED to ensure the failure is exposed.
- Around line 186-205: The SQL score uses UNIX_TIMESTAMP() which is constant per
query and doesn't provide a per-row deterministic tiebreaker; remove the
UNIX_TIMESTAMP() term from the score expression constructed in
ProductRankingMvJobConfig (the multi-line String sql formatted with w.view(),
w.like(), w.order()) and instead add a deterministic tie-breaker to the
ROW_NUMBER() expression in mergeStep (e.g., include product_id or another stable
column in the ORDER BY used for ROW_NUMBER) so equal scores are broken
consistently; also add a test that seeds two products with identical metrics and
runs the ranking job twice (idempotentDoubleExecution variant) asserting
identical ranking outputs across runs.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/step/CleanupTasklet.java`:
- Around line 46-47: CleanupTasklet의 execute 진입 직후 targetDate를 DATE_FORMATTER로
파싱해 유효성 검증하고 cutoffDate/cutoffKey 계산을 DELETE 작업들보다 먼저 수행하여
DateTimeParseException이 DELETE 이후에 발생하지 않도록 하세요; 구체적으로 execute 메서드에서 targetDate
문자열을 LocalDate.parse(targetDate, DATE_FORMATTER)로 즉시 시도(또는 try-catch로 검증)하여 잘못된
포맷이면 Job 실패를 명확히 반환하고, cutoffDate · cutoffKey · RETENTION_DAYS 계산을 모든 delete
호출(예: 첫 번째 DELETE 이전)에 사용할 수 있도록 이동 또는 선계산하세요.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`:
- Around line 294-309: The test idempotentDoubleExecution currently only asserts
COUNT(*) and misses verifying identical results; modify it to capture the full
result set from mv_product_rank_weekly after the first run and again after the
second run and assert the two sets are equal. Specifically, after
seedProducts/seedMetrics and the first runJob("weekly"), query
mv_product_rank_weekly for (product_id, ranking, score) for period_key =
TARGET_DATE (use jdbcTemplate to fetch a list/rows), store that result, run
runJob("weekly") a second time, fetch the same (product_id, ranking, score) set
again, and assert equality of the two collections (and still assert second
BatchStatus == BatchStatus.COMPLETED). Ensure comparisons treat ordering/ties
deterministically (compare as sets or sort by product_id) and include
product_id, ranking and score in the assertion.
- Around line 638-664: The test cancellationReflectedInScore in
ProductRankingMvJobE2ETest hardcodes '2026-04-16', breaking sync with
TARGET_DATE; change the two INSERTs to derive the ISO date string from
TARGET_DATE (e.g., parse TARGET_DATE (yyyyMMdd) to a LocalDate and format as
yyyy-MM-dd) so the metric_date used in jdbcTemplate.update matches
TARGET_DATE-driven windows, update both product INSERT statements in
cancellationReflectedInScore (and any similar tests), and optionally add a new
edge test verifying when cancel_amount >= sales_amount the net sales floor to 0
(GREATEST(..., 0)).
- Around line 561-634: The E2E test
ProductRankingMvJobE2ETest.largeScalePartitionedBatchTest is too heavy for
normal CI and its batch inserts (seedProductsBulk, seedMetricsBulkWithTrends)
suffer from JDBC batch rewrite missing; mark the test as isolated by adding
`@Tag`("slow") and `@Timeout`(minutes = 10) (or guard with
`@EnabledIfEnvironmentVariable`("PERF_TEST_ENABLED","true")) on the class or
method, and enable rewriteBatchedStatements=true for the test datasource (via
application-test.yml or a `@TestConfiguration` that configures the DataSource used
by jdbcTemplate) so jdbcTemplate.batchUpdate will be rewritten into multi-value
INSERTs; after applying these changes run the test once to adjust the timeout
value as needed.
In `@apps/commerce-batch/src/test/resources/schema-batch-test.sql`:
- Around line 147-156: The mv_product_rank_staging table's primary key is
(product_id, period_key) which prevents range scans on period_key alone and
causes full table scans in CleanupTasklet (DELETE ... WHERE period_key = ?) and
mergeStep (SELECT ... WHERE period_key = ? ORDER BY score DESC LIMIT 100); add a
secondary index on period_key (e.g., CREATE INDEX ON
mv_product_rank_staging(period_key)) to ensure those queries use an index. After
adding the index, validate in largeScalePartitionedBatchTest by running EXPLAIN
for the cleanup and merge queries to confirm index usage (type=range or ref) and
measure cleanup/merge step durations to detect regressions.
- Around line 119-145: Add a schema-level UNIQUE constraint on (period_key,
product_id) for both mv_product_rank_weekly and mv_product_rank_monthly to
prevent duplicate product rows per period; update the DDL in this test file (and
corresponding production DDL) to include UNIQUE(period_key, product_id) (or a
named constraint/index) and run tests that simulate cleanupStep success followed
by two mergeStep executions (or merge retry with cleanup skipped) to ensure
duplicate inserts fail and are detected; reference the tables
mv_product_rank_weekly, mv_product_rank_monthly, the CleanupTasklet, mergeStep,
and the findByPeriodKeyOrderByRankingAsc query when adding the constraint and
test.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 76-77: Add defensive validation at the start of the RankingFacade
method that computes pagination: check that page >= 0 and size > 0 and if not
throw new CoreException(ErrorType.BAD_REQUEST). Ensure this validation runs
before calling PageRequest.of(page, size) and before computing totalPages (the
lines with totalElements = Math.min(totalElements, MAX_RANKING_SIZE) and
totalPages = (int) Math.ceil((double) totalElements / size)) so you avoid
ArithmeticException or inconsistent errors from PageRequest.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.java`:
- Around line 13-17: Add a consistent identifier-based equality policy to
MvProductRank by implementing equals and hashCode using the private Long id
field: locate the MvProductRank class and either annotate it with Lombok
`@EqualsAndHashCode`(of = "id") or add manual implementations that return true
only when both objects are the same class and non-null ids are equal, and treat
entities with null id as not equal (and compute hashCode accordingly). Ensure
the implementations are placed on the MvProductRank abstract class so all
subclasses inherit the id-based equality semantics.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.java`:
- Around line 7-12: 현재 MvProductRankRepository의 메서드 시그니처
findByPeriodKeyAndScope(String periodKey, String scope, Pageable pageable)가 문자열로
scope를 노출해 호출측 오타/대소문자 문제를 유발하므로, 도메인 안전성을 위해 scope 타입을 String에서 새로운 enum
RankingScope(WEEKLY, MONTHLY 등)으로 바꿔 시그니처를 findByPeriodKeyAndScope(String
periodKey, RankingScope scope, Pageable pageable)로 변경하고,
MvProductRankJpaRepository의 switch/처리 로직을 RankingScope 기반으로 리팩토링하며 외부에서 들어오는 원시
문자열은 경계에 위치한 파서(예: RankingKeyResolver)에서 변환하고 잘못된 값은 CoreException으로 변환해 처리하도록
변경 및 해당 변환 실패 케이스를 검증하는 테스트를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankJpaRepository.java`:
- Around line 17-35: Consolidate the duplicate weekly/monthly branching in
MvProductRankJpaRepository by validating and normalizing scope into a single
enum (e.g., ScopeType with WEEKLY/MONTHLY) at repository entry (or have the
Facade throw CoreException(ErrorType.BAD_REQUEST) on invalid scope) and then
switch once to select the correct repository (weeklyRepository or
monthlyRepository) for both findByPeriodKeyAndScope and
countByPeriodKeyAndScope; remove the unnecessary .stream().map(...) casts when
returning List<MvProductRank> and ensure invalid scope paths raise a controlled
CoreException or accept only ScopeType to guarantee 4xx behavior for unsupported
scope strings.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlySpringDataRepository.java`:
- Around line 11-13: 현재 findByPeriodKeyOrderByRankingAsc(String periodKey,
Pageable pageable)는 ranking 동률 발생 시 페이지 정합성이 깨질 위험이 있으니 페이징 tiebreaker를 추가해
안정화하세요: repository에서 메서드명을 findByPeriodKeyOrderByRankingAscIdAsc(...)로 변경해
ranking에 이어 id(또는 productId)를 오름차순으로 추가 정렬하고, 필요하면 DB 인덱스도 (period_key, ranking,
id)로 확장하세요; 또한 페이징 경계 케이스를 검사하는 테스트(동일 ranking 값이 2건 이상인 고정 데이터로 페이지 사이즈 경계에서
누락/중복 없음)를 추가해 검증하세요.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java`:
- Around line 111-113: The days calculation for scope is duplicated between
createPartitioner and stagingReader; extract the logic into a single helper
(e.g., a private static method windowDays(String scope) or a constant lookup)
and replace both occurrences (the ternary int days = "weekly".equals(scope) ? 6
: 29;) with calls to windowDays(scope) so both createPartitioner and
stagingReader use the same source of truth for the partition/aggregation window.
- Around line 259-287: The tasklet for mergeStep injects a table name via
String.format/switch which is fragile and duplicated with CleanupTasklet;
replace the ad-hoc switch in mergeStep with a single source of truth (e.g., a
module-level Map<String,String> or an enum like ScopeType with a mvTable()
method) and call that resolver (reuse CleanupTasklet#resolveMvTable or move it
into a shared helper) instead of inline formatting, make the tasklet
registration consistent with CleanupTasklet (both as `@Bean/`@JobScope or both as
components), and add a deterministic tiebreaker to the INSERT ... SELECT ...
ORDER BY score DESC (e.g., ORDER BY score DESC, product_id) to ensure stable
LIMIT 100 results.
- Around line 109-128: The createPartitioner method currently returns a single
dummy partition ("partition0" with minProductId=0 and maxProductId=0) when
productIds.isEmpty(), which creates only one worker instead of honoring
gridSize; change the empty-data branch in createPartitioner to return an empty
Map (e.g., Collections.emptyMap() or Map.of()) and update the log to be explicit
(e.g., log.warn("[Partitioner] no data for date range {} ~ {}, returning 0
partitions", startDate, endDate)); this preserves the job COMPLETED behavior in
tests while making partitioning and observability correct and allows upstream
flow/step logic to detect “no partitions” and handle the no-data case.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/step/CleanupTasklet.java`:
- Around line 34-59: The retention purge ("period_key < cutoffKey") in
CleanupTasklet.execute should be separated from the exact-match deletes to avoid
long-running single transactions: either move the cutoff deletes into a
dedicated Step/Tasklet (e.g., new RetentionPurgeTasklet) and call it after the
exact-match step, or implement chunked deletes inside CleanupTasklet.execute by
looping deletes with "DELETE FROM <table> WHERE period_key < ? LIMIT N" for both
resolveMvTable(scope) and mv_product_rank_staging, committing between chunks;
reference the symbols CleanupTasklet.execute, resolveMvTable(scope),
mv_product_rank_staging, RETENTION_DAYS, DATE_FORMATTER and ensure logs still
report purged counts and timings per chunk or in the new Tasklet.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`:
- Around line 313-326: The test noDataProducesEmptyMv currently only asserts
BatchStatus.COMPLETED and that mv_product_rank_weekly has zero rows for
TARGET_DATE; extend it to also assert that mv_product_rank_staging has zero rows
for the same period_key and verify that rows from previous periods outside the
retention boundary are not present (i.e., query mv_product_rank_weekly for the
previous period_key/retention boundary and assert count is zero); update the
assertions after calling runJob("weekly") to include these two extra checks so
the Partitioner empty-partition ("0,0") and retention-cleanup paths are
validated.
- Around line 57-64: The current seedProducts method performs single-row JDBC
updates in a loop causing many round-trips; change it to use a batch insert
(e.g., call or mirror seedProductsBulk) so tests like weeklySuccess,
monthlySuccess, and idempotentDoubleExecution use jdbcTemplate.batchUpdate (or
reuse seedProductsBulk) to insert all products in one JDBC call, preserving the
same columns/values and parameter order as the existing per-row SQL; update the
seedProducts method body to build the batch params and call batchUpdate to
reduce round-trips and test runtime.
- Around line 344-557: This test (printRankingResults) has no assertions and
should be marked as a visualization-only test and given a few smoke assertions
to detect regressions: add `@Tag`("visualization") (or `@Disabled`) to the
printRankingResults method and after populating data and running
runJob("weekly")/runJob("monthly") extract the compareRows results to assert
representative product rank relationships (e.g., for product ids 1-3 assert
daily_rank < weekly_rank (recent spike => better daily), for ids 17-20 assert
monthly_rank <= weekly_rank <= daily_rank (long-term strong), for ids 14-15
assert weekly_rank < daily_rank (recent decline), and for id 8 assert daily_rank
<< weekly_rank/monthly_rank (today viral)). When asserting values, use the rows
returned from compareRows and the mv_product_rank_weekly /
mv_product_rank_monthly entries (w.ranking / mo.ranking) rather than relying on
d.ranking ordering if you need MV-specific ordering.
In `@build.gradle.kts`:
- Line 85: The global jvmArgs("-Xshare:off", "-Xmx2g") in the subprojects block
should be removed and the 2G heap setting applied only to the module that needs
it (project(":apps:commerce-batch")) or to the specific test task containing
ProductRankingMvJobE2ETest; update build.gradle.kts to stop setting -Xmx2g in
subprojects and instead add a tasks.test { jvmArgs("-Xshare:off","-Xmx2g") } (or
equivalent test task configuration) inside the commerce-batch project (or the
project that contains ProductRankingMvJobE2ETest) so other modules keep the
default heap and only the heavy E2E test uses 2G.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: acda5740-0ab9-498e-ae7c-af0c4eadc54e
⛔ Files ignored due to path filters (23)
docs/captures/01-event-flow.pngis excluded by!**/*.png,!**/*.pngand included by**docs/captures/02-drift-initial.pngis excluded by!**/*.png,!**/*.pngand included by**docs/captures/03-ranking-mv-test-output.mdis excluded by!**/*.mdand included by**docs/captures/04-ranking-api-capture.mdis excluded by!**/*.mdand included by**docs/design/10-technical-writing-plan.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-batch-analysis-report.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-batch-code-reference.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-batch-ranking-system.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-batch-test-results.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-large-scale-test-prompt.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-partition-benchmark-prompt.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-pr-draft.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-technical-writing-topics.mdis excluded by!**/*.mdand included by**docs/design/volume-10/11-ranking-batch-test-blog.mdis excluded by!**/*.mdand included by**docs/design/volume-9/09-event-review.mdis excluded by!**/*.mdand included by**docs/design/volume-9/09-ranking-system-design.mdis excluded by!**/*.mdand included by**docs/design/volume-9/09-ranking-system.mdis excluded by!**/*.mdand included by**docs/requirements/volume-10/10-batch-ranking-learning.mdis excluded by!**/*.mdand included by**docs/requirements/volume-10/10-batch-ranking-progress.mdis excluded by!**/*.mdand included by**docs/requirements/volume-10/10-batch-ranking-quests.mdis excluded by!**/*.mdand included by**docs/session-prompts/10-batch-analysis-prompt.mdis excluded by!**/*.mdand included by**docs/session-prompts/10-batch-tutor-prompt.mdis excluded by!**/*.mdand included by**docs/velog-techwriting-vol10.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (14)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlySpringDataRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklySpringDataRepository.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/step/CleanupTasklet.javaapps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.javaapps/commerce-batch/src/test/resources/schema-batch-test.sqlbuild.gradle.ktsmodules/jpa/src/testFixtures/java/com/loopers/testcontainers/MySqlTestContainersConfig.java
Comment on lines
43
to
+50
| public RankingDto.PagedRankingResponse getRankings(String scope, String date, int page, int size, Long memberId) { | ||
| String resolvedDate = (date != null) ? date : LocalDate.now(KST).format(DATE_FORMATTER); | ||
| String prefix = resolveZsetPrefix(scope, memberId); | ||
|
|
||
| return switch (scope) { | ||
| case "weekly", "monthly" -> getFromMv(scope, resolvedDate, page, size); | ||
| default -> getFromRedis(scope, resolvedDate, page, size, memberId); | ||
| }; | ||
| } |
There was a problem hiding this comment.
scope가 null로 들어올 때 NPE가 발생한다.
switch (scope)에서 scope가 null이면 NullPointerException이 발생해 500으로 떨어진다. 컨트롤러 계층에서 필터링되고 있더라도, 이 Facade는 memberId/date에 대해서는 null을 방어적으로 처리하는 반면 scope만 방어가 없어 일관성이 떨어진다.
🛡️ 제안 수정
- return switch (scope) {
+ String resolvedScope = scope == null ? "" : scope;
+ return switch (resolvedScope) {
case "weekly", "monthly" -> getFromMv(scope, resolvedDate, page, size);
default -> getFromRedis(scope, resolvedDate, page, size, memberId);
};혹은 ScopeType enum을 도입하여 상위 계층에서 검증하도록 통일하는 편이 낫다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 43 - 50, getRankings currently does a switch(scope) which throws
NPE when scope is null; guard against null before the switch in
RankingFacade.getRankings by resolving scope to a safe value or validating it
(e.g., if scope == null -> use a default like "default" or call the same
fallback path as the switch default) and then perform the switch, or explicitly
throw a clear IllegalArgumentException; alternatively consider introducing a
ScopeType enum and require/convert scope to that enum before branching so the
null/invalid case is handled consistently with memberId/date checks and routed
to getFromRedis/getFromMv as appropriate.
Comment on lines
+52
to
+74
| private RankingDto.PagedRankingResponse getFromMv(String scope, String date, int page, int size) { | ||
| // 1. 당일 MV 조회 | ||
| List<MvProductRank> mvResults = mvProductRankRepository.findByPeriodKeyAndScope( | ||
| date, scope, PageRequest.of(page, size)); | ||
| long totalElements = mvProductRankRepository.countByPeriodKeyAndScope(date, scope); | ||
|
|
||
| // 2. 당일 데이터 없으면 전일 fallback | ||
| if (mvResults.isEmpty()) { | ||
| String previousDate = LocalDate.parse(date, DATE_FORMATTER) | ||
| .minusDays(1).format(DATE_FORMATTER); | ||
| mvResults = mvProductRankRepository.findByPeriodKeyAndScope( | ||
| previousDate, scope, PageRequest.of(page, size)); | ||
| totalElements = mvProductRankRepository.countByPeriodKeyAndScope(previousDate, scope); | ||
|
|
||
| if (!mvResults.isEmpty()) { | ||
| log.info("MV 전일 fallback 적용: scope={}, date={} → {}", scope, date, previousDate); | ||
| } | ||
| } | ||
|
|
||
| // 3. 전일도 없으면 빈 결과 | ||
| if (mvResults.isEmpty()) { | ||
| return new RankingDto.PagedRankingResponse(List.of(), 0, 0, page, size); | ||
| } |
There was a problem hiding this comment.
Fallback 트리거 조건이 잘못되어 정상 데이터가 전일 데이터로 덮어써질 수 있다.
mvResults.isEmpty()는 "해당 period에 데이터가 없음"과 "요청한 page가 범위 밖"을 구분하지 못한다. 예를 들어 오늘 MV에 100건이 존재하고 클라이언트가 page=5, size=50으로 요청하면, findByPeriodKeyOrderByRankingAsc는 오프셋이 총건수를 초과하여 빈 리스트를 반환한다. 이 상황에서 현재 구현은 "오늘 데이터가 없다"고 오판하여 전일(previousDate) 결과로 mvResults와 totalElements를 모두 덮어써, 오늘 랭킹이 멀쩡히 존재함에도 API 응답이 전일 데이터를 섞어 내보낸다. 운영에서 MV 전일 fallback 적용 로그만 남으므로 원인 추적도 어렵다.
Fallback 트리거는 반드시 count 기준으로 판단해야 하며, 호출 순서도 count 먼저 조회하도록 정리하는 것이 낫다.
🐛 제안 수정
private RankingDto.PagedRankingResponse getFromMv(String scope, String date, int page, int size) {
- // 1. 당일 MV 조회
- List<MvProductRank> mvResults = mvProductRankRepository.findByPeriodKeyAndScope(
- date, scope, PageRequest.of(page, size));
- long totalElements = mvProductRankRepository.countByPeriodKeyAndScope(date, scope);
-
- // 2. 당일 데이터 없으면 전일 fallback
- if (mvResults.isEmpty()) {
- String previousDate = LocalDate.parse(date, DATE_FORMATTER)
- .minusDays(1).format(DATE_FORMATTER);
- mvResults = mvProductRankRepository.findByPeriodKeyAndScope(
- previousDate, scope, PageRequest.of(page, size));
- totalElements = mvProductRankRepository.countByPeriodKeyAndScope(previousDate, scope);
-
- if (!mvResults.isEmpty()) {
- log.info("MV 전일 fallback 적용: scope={}, date={} → {}", scope, date, previousDate);
- }
- }
-
- // 3. 전일도 없으면 빈 결과
- if (mvResults.isEmpty()) {
- return new RankingDto.PagedRankingResponse(List.of(), 0, 0, page, size);
- }
+ // 1. 당일 총건수로 존재 여부 판단 (빈 페이지 ≠ 데이터 없음)
+ String effectiveDate = date;
+ long totalElements = mvProductRankRepository.countByPeriodKeyAndScope(effectiveDate, scope);
+
+ // 2. 당일 데이터 자체가 없을 때만 전일 fallback
+ if (totalElements == 0) {
+ String previousDate = LocalDate.parse(date, DATE_FORMATTER)
+ .minusDays(1).format(DATE_FORMATTER);
+ long prevTotal = mvProductRankRepository.countByPeriodKeyAndScope(previousDate, scope);
+ if (prevTotal == 0) {
+ return new RankingDto.PagedRankingResponse(List.of(), 0, 0, page, size);
+ }
+ log.info("MV 전일 fallback 적용: scope={}, date={} → {}", scope, date, previousDate);
+ effectiveDate = previousDate;
+ totalElements = prevTotal;
+ }
+
+ List<MvProductRank> mvResults = mvProductRankRepository.findByPeriodKeyAndScope(
+ effectiveDate, scope, PageRequest.of(page, size));추가 테스트로 다음 케이스를 권한다.
- 당일 MV 100건 존재 +
page=5, size=50요청 시 전일 fallback이 트리거되지 않고 빈 data/정상 totalElements를 반환 - 당일 MV 0건 + 전일 MV 100건 +
page=0요청 시 전일 데이터로 fallback되고 로그가 남음 - 당일/전일 모두 0건일 때 빈 응답
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 52 - 74, The fallback currently uses mvResults.isEmpty() which
conflates "no data for period" with "page out of range" and can wrongly replace
today's data with yesterday's; in getFromMv, first call
mvProductRankRepository.countByPeriodKeyAndScope(date, scope) to get
totalElements and only if totalElements == 0 perform the previousDate fallback
(recompute previousDate, set totalElements =
countByPeriodKeyAndScope(previousDate, scope) and log), then call
mvProductRankRepository.findByPeriodKeyAndScope(...) to load mvResults using
PageRequest.of(page, size); ensure you reference and update totalElements and
mvResults only when count indicates zero so that page-out-of-range cases return
empty results for the existing period instead of falling back.
Comment on lines
+43
to
+44
| @Column(nullable = false) | ||
| private LocalDateTime createdAt; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# MvProductRank(Weekly|Monthly) 가 JPA save/persist 경로로 쓰이는지 확인
rg -nP --type=java -C3 '\b(MvProductRankWeekly|MvProductRankMonthly)\b'
rg -nP --type=java -C2 '\b(save|persist)\s*\(' -g '**/ranking/**'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5490
🏁 Script executed:
cat apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1026
🏁 Script executed:
# 배치 Job 구현 찾기
fd -e java -path '**/ranking/**' | head -20
rg -n '@PrePersist|@CreationTimestamp' -t java -g '**/ranking/**'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 253
🏁 Script executed:
# 전체 저장소에서 save/persist 호출 찾기
rg -n '\.(save|saveAll|persist)\s*\(' -t java --max-count 50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 40751
🏁 Script executed:
# 랭킹 관련 배치 Job 또는 JDBC 직접 INSERT 찾기
fd -e java -path '**/ranking/**' -o -path '**/batch/**' -o -path '**/job/**' | head -30
rg -n 'MvProductRank|MERGE|INSERT' -A5 -B5 --type=java | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 13939
🏁 Script executed:
# DB 마이그레이션 파일에서 mv_product_rank 스키마 확인
fd -e sql -o -e yaml -o -e xml | xargs grep -l 'mv_product_rank' 2>/dev/null | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 201
JPA 저장 경로 추가 시 createdAt 자동 설정 부재로 인한 제약 위반 위험 존재.
현재는 배치 Job이 JDBC 기반 INSERT로 createdAt 값을 명시적으로 채우고 있어 문제가 발생하지 않으나, JpaRepository 인터페이스 존재로 인해 향후 누군가 save() 메서드로 이 엔티티를 영속화하면 createdAt이 NULL이 되어 ConstraintViolationException이 발생한다. 도메인 모델 자체에 자동 설정 메커니즘을 추가하여 방어해야 한다.
- 왜 문제인가: JPA 저장 경로가 추가되는 순간 즉시 저장 실패가 발생하며, DB 스키마의 DEFAULT CURRENT_TIMESTAMP도 하이버네이트의 INSERT 시 NULL 전달로 무효화된다.
- 수정안:
@PrePersist로 기본값을 채우거나, 하이버네이트의@CreationTimestamp를 부여한다.
♻️ 제안
- `@Column`(nullable = false)
- private LocalDateTime createdAt;
+ `@Column`(nullable = false, updatable = false)
+ private LocalDateTime createdAt;
+
+ `@PrePersist`
+ protected void onCreate() {
+ if (createdAt == null) {
+ createdAt = LocalDateTime.now();
+ }
+ }- 추가 테스트: JPA
save()경로로 엔티티를 저장하는 리포지토리 단위 테스트를 추가해createdAt자동 설정 여부를 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRank.java`
around lines 43 - 44, MvProductRank currently declares private LocalDateTime
createdAt with `@Column`(nullable = false) but has no JPA-side auto-population, so
saving via JpaRepository will set NULL and violate the constraint; update the
entity to auto-populate createdAt (either add a `@PrePersist` method in
MvProductRank that sets createdAt = LocalDateTime.now() if null, or annotate the
field with Hibernate's `@CreationTimestamp`) and add a repository unit test that
saves MvProductRank via save() to assert createdAt is non-null after persist.
Comment on lines
+75
to
+83
| return new JobBuilder(JOB_NAME, jobRepository) | ||
| .incrementer(new RunIdIncrementer()) | ||
| .start(cleanupStep).on("FAILED").end() | ||
| .from(cleanupStep).on("*").to(partitionedAggregateStep) | ||
| .next(mergeStep) | ||
| .end() | ||
| .listener(jobListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring Batch FlowBuilder end() vs fail() final BatchStatus
💡 Result:
In Spring Batch's FlowBuilder.TransitionBuilder, the end and fail methods define terminal transitions for a flow based on a step's or decider's exit status pattern. - end: Signals the successful end of the flow by transitioning to an EndState with FlowExecutionStatus.COMPLETED. This sets the flow's execution status to COMPLETED. - fail: Signals the end of the flow with an error condition by transitioning to a pre-defined EndState with FlowExecutionStatus.FAILED (initialized as new EndState(FlowExecutionStatus.FAILED, prefix + "FAILED")). This sets the flow's execution status to FAILED. When the Flow is executed (e.g., as part of a Job via FlowJob), the final BatchStatus of the JobExecution is derived from the Flow's FlowExecutionStatus: - If COMPLETED, the Job's BatchStatus is COMPLETED (non-restartable). - If FAILED, the Job's BatchStatus is FAILED (restartable). This mirrors the XML equivalents (BatchStatus COMPLETED, non-restartable) and (BatchStatus FAILED, restartable). The FlowExecutionStatus determines if the flow/job ends successfully or in a failed-but-restartable state, independent of individual step statuses.
Citations:
- 1: https://docs.spring.io/spring-batch/reference/step/controlling-flow.html
- 2: https://docs.spring.io/spring-batch/docs/current/api/org/springframework/batch/core/job/builder/FlowBuilder.TransitionBuilder.html
- 3: https://github.com/spring-projects/spring-batch/blob/master/spring-batch-core/src/main/java/org/springframework/batch/core/job/builder/FlowBuilder.java
- 4: https://github.com/spring-projects/spring-batch/blob/master/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/FlowExecutionStatus.java
cleanupStep 실패 시 Job이 COMPLETED로 종료되어 장애가 은폐된다.
.start(cleanupStep).on("FAILED").end() 구문에서 Spring Batch의 .end() 메서드는 FlowExecutionStatus를 COMPLETED로 설정하므로, cleanup이 실패해도 Job의 최종 BatchStatus가 COMPLETED로 기록된다. 이는 운영 모니터링 시스템이 실패를 감지하지 못하고, MV와 staging 테이블에 기존 데이터가 일부만 정리된 상태로 남아 정합성이 깨지는 장애로 이어진다.
cleanupStep 실패를 명확히 노출하려면 .fail()을 사용해야 Job의 최종 BatchStatus가 FAILED로 기록되고, 운영 알림 파이프라인이 정상 작동한다.
제안 수정
return new JobBuilder(JOB_NAME, jobRepository)
.incrementer(new RunIdIncrementer())
- .start(cleanupStep).on("FAILED").end()
+ .start(cleanupStep).on("FAILED").fail()
.from(cleanupStep).on("*").to(partitionedAggregateStep)
.next(mergeStep)
.end()
.listener(jobListener)
.build();CleanupTasklet이 의도적으로 예외를 던지도록 주입한 후 jobLauncherTestUtils.launchJob(...)의 BatchStatus가 FAILED로 반환되는지 확인하는 E2E 테스트 1건을 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java`
around lines 75 - 83, The job flow currently ends with
.start(cleanupStep).on("FAILED").end(), which marks the Job as COMPLETED when
cleanupStep fails; update the transition to use .fail() instead of .end() so a
cleanupStep failure results in a FAILED JobStatus (modify the JobBuilder flow in
ProductRankingMvJobConfig to replace .end() with .fail() for the "FAILED" branch
of cleanupStep). Additionally add one E2E test that injects a CleanupTasklet
that throws, runs the job via jobLauncherTestUtils.launchJob(...), and asserts
the returned BatchStatus is FAILED to ensure the failure is exposed.
Comment on lines
+186
to
+205
| String sql = """ | ||
| SELECT | ||
| pm.product_id, | ||
| SUM(pm.view_count) AS total_view_count, | ||
| SUM(pm.like_count - pm.unlike_count) AS total_net_like_count, | ||
| SUM(pm.sales_count) AS total_sales_count, | ||
| SUM(pm.sales_amount - pm.cancel_amount_by_event_date) AS total_net_sales_amount, | ||
| ( | ||
| %s * LOG10(GREATEST(SUM(pm.view_count), 0) + 1) / 7.0 | ||
| + %s * LOG10(GREATEST(SUM(pm.like_count - pm.unlike_count), 0) + 1) / 7.0 | ||
| + %s * LOG10(GREATEST(SUM(pm.sales_amount - pm.cancel_amount_by_event_date), 0) + 1) / 7.0 | ||
| + UNIX_TIMESTAMP() * 1e-16 | ||
| ) AS score | ||
| FROM product_metrics pm | ||
| JOIN product p ON pm.product_id = p.id | ||
| WHERE pm.metric_date BETWEEN ? AND ? | ||
| AND pm.product_id BETWEEN ? AND ? | ||
| AND p.deleted_at IS NULL | ||
| GROUP BY pm.product_id | ||
| """.formatted(w.view(), w.like(), w.order()); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MySQL UNIX_TIMESTAMP() evaluated per row or per statement
💡 Result:
MySQL's UNIX_TIMESTAMP (when called with no arguments) is evaluated once per statement (query), not per row. Multiple invocations within the same SELECT, INSERT, UPDATE, or other statement return the same value, corresponding to the time at the start of statement execution. This behavior matches other current date/time functions like NOW, CURRENT_TIMESTAMP, etc., which the official MySQL documentation explicitly states are "evaluated only once per query at the start of query execution." Although not always listed explicitly with NOW, empirical tests confirm UNIX_TIMESTAMP follows the same rule. Example test query: SELECT UNIX_TIMESTAMP AS t1, SLEEP(2), UNIX_TIMESTAMP AS t2; Both t1 and t2 show the same timestamp despite the 2-second delay. In multi-row operations like INSERT...SELECT with 1000 rows, all rows receive identical UNIX_TIMESTAMP values. For per-row unique timestamps, use SYSDATE (which evaluates at invocation time) or application-side generation. Note: UNIX_TIMESTAMP(date) with an argument evaluates based on that specific date (deterministic if argument is fixed), but the no-arg version for current time is statement-stable but nondeterministic across statements due to real-time changes.
Citations:
- 1: https://dev.mysql.com/doc/refman/9.0/en/date-and-time-functions.html
- 2: https://dev.mysql.com/doc/en/date-and-time-functions.html
- 3: https://dev.mysql.com/doc/refman/en/date-and-time-functions.html
- 4: https://stackoverflow.com/questions/33520066/is-unix-timestamp-calculated-once-for-large-insert-queries
- 5: https://stackoverflow.com/questions/28915848/unix-timestamp-repeated-more-than-once-in-mysql-request
- 6: https://stackoverflow.com/questions/31946784/phps-time-vs-mysqls-unit-timestamp-in-where-clause-which-is-faster
- 7: https://stackoverflow.com/questions/13886286/duplicate-mysql-unix-timestamp-result-on-successive-calls
UNIX_TIMESTAMP() * 1e-16 표현식이 의도한 타이브레이크 역할을 하지 못한다.
MySQL에서 UNIX_TIMESTAMP()는 한 SELECT 쿼리 내에서 상수로 한 번 평가되므로, 같은 쿼리의 모든 행에 동일한 값이 더해진다. 즉, 동점 상품들 간에는 여전히 동일 score가 되어 타이브레이크가 발생하지 않고, 대신 실행 시각에 따라 전체 score 값이 1e-7 수준으로 흔들려 실행 간 score 재현성이 떨어진다. 이는 운영 환경에서 score 기반 대조, 회귀 비교를 불가능하게 한다. 해당 테스트(idempotentDoubleExecution)는 개수만 비교하므로 통과하지만 실제 ranking 일관성을 보장하지 못한다.
타이브레이크가 필요하다면 행별로 결정적인 값을 사용해야 한다.
수정 제안
(
%s * LOG10(GREATEST(SUM(pm.view_count), 0) + 1) / 7.0
+ %s * LOG10(GREATEST(SUM(pm.like_count - pm.unlike_count), 0) + 1) / 7.0
+ %s * LOG10(GREATEST(SUM(pm.sales_amount - pm.cancel_amount_by_event_date), 0) + 1) / 7.0
- + UNIX_TIMESTAMP() * 1e-16
) AS scoremergeStep의 ROW_NUMBER 절에 결정적 tie-breaker를 추가한다:
- ROW_NUMBER() OVER (ORDER BY score DESC) AS ranking,
+ ROW_NUMBER() OVER (ORDER BY score DESC, product_id ASC) AS ranking,추가 테스트로, 동일한 metric을 가진 product 2건을 시드한 후 동일 조건으로 2회 이상 실행하여 ranking 배치 결과가 매번 동일한지 검증하는 케이스를 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java`
around lines 186 - 205, The SQL score uses UNIX_TIMESTAMP() which is constant
per query and doesn't provide a per-row deterministic tiebreaker; remove the
UNIX_TIMESTAMP() term from the score expression constructed in
ProductRankingMvJobConfig (the multi-line String sql formatted with w.view(),
w.like(), w.order()) and instead add a deterministic tie-breaker to the
ROW_NUMBER() expression in mergeStep (e.g., include product_id or another stable
column in the ORDER BY used for ROW_NUMBER) so equal scores are broken
consistently; also add a test that seeds two products with identical metrics and
runs the ranking job twice (idempotentDoubleExecution variant) asserting
identical ranking outputs across runs.
Comment on lines
+294
to
+309
| @Test | ||
| @DisplayName("멱등성 — 같은 파라미터로 2회 실행해도 결과 동일") | ||
| void idempotentDoubleExecution() throws Exception { | ||
| seedProducts(50); | ||
| seedMetrics(50, 7, TARGET_DATE); | ||
|
|
||
| runJob("weekly"); | ||
| BatchStatus secondStatus = runJob("weekly"); | ||
|
|
||
| assertThat(secondStatus).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| int mvCount = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(mvCount).isEqualTo(50); | ||
| } |
There was a problem hiding this comment.
멱등성 테스트가 개수만 비교하여 실제 멱등성을 검증하지 못한다.
현 테스트는 2회 실행 후 COUNT(*)=50만 확인하나, 이는 CleanupTasklet의 DELETE가 동작했음만 보여줄 뿐 "동일 입력이면 동일 랭킹/점수가 나오는가"는 검증하지 못한다. 앞서 지적한 UNIX_TIMESTAMP() * 1e-16 이슈나 tie-breaker 부재가 실제 운영 데이터에서 rank 역전을 유발해도 본 테스트는 통과한다.
(product_id, ranking, score)의 전체 집합을 1회차/2회차로 비교하는 어서션을 권한다.
✅ 제안 수정
- runJob("weekly");
- BatchStatus secondStatus = runJob("weekly");
-
- assertThat(secondStatus).isEqualTo(BatchStatus.COMPLETED);
-
- int mvCount = jdbcTemplate.queryForObject(
- "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE period_key = ?",
- Integer.class, TARGET_DATE);
- assertThat(mvCount).isEqualTo(50);
+ runJob("weekly");
+ var first = jdbcTemplate.queryForList(
+ "SELECT product_id, ranking FROM mv_product_rank_weekly WHERE period_key = ? ORDER BY ranking",
+ TARGET_DATE);
+
+ BatchStatus secondStatus = runJob("weekly");
+ assertThat(secondStatus).isEqualTo(BatchStatus.COMPLETED);
+
+ var second = jdbcTemplate.queryForList(
+ "SELECT product_id, ranking FROM mv_product_rank_weekly WHERE period_key = ? ORDER BY ranking",
+ TARGET_DATE);
+ assertThat(second).hasSize(50).isEqualTo(first);[as_per_coding_guidelines] 가이드라인 "단위/통합 테스트는 경계/실패 케이스/플래키 가능성"에 따른 보강 제안이다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`
around lines 294 - 309, The test idempotentDoubleExecution currently only
asserts COUNT(*) and misses verifying identical results; modify it to capture
the full result set from mv_product_rank_weekly after the first run and again
after the second run and assert the two sets are equal. Specifically, after
seedProducts/seedMetrics and the first runJob("weekly"), query
mv_product_rank_weekly for (product_id, ranking, score) for period_key =
TARGET_DATE (use jdbcTemplate to fetch a list/rows), store that result, run
runJob("weekly") a second time, fetch the same (product_id, ranking, score) set
again, and assert equality of the two collections (and still assert second
BatchStatus == BatchStatus.COMPLETED). Ensure comparisons treat ordering/ties
deterministically (compare as sets or sort by product_id) and include
product_id, ranking and score in the assertion.
Comment on lines
+561
to
+634
| @Test | ||
| @DisplayName("대규모 — 10만 상품 × 30일 메트릭, 4 Partition 병렬 집계") | ||
| void largeScalePartitionedBatchTest() throws Exception { | ||
| int productCount = 100_000; | ||
| int metricDays = 30; | ||
|
|
||
| // ── 시드 ── | ||
| long t0 = System.currentTimeMillis(); | ||
| seedProductsBulk(productCount); | ||
| long productSeedMs = System.currentTimeMillis() - t0; | ||
|
|
||
| t0 = System.currentTimeMillis(); | ||
| seedMetricsBulkWithTrends(productCount, metricDays, TARGET_DATE); | ||
| long metricSeedMs = System.currentTimeMillis() - t0; | ||
|
|
||
| int metricRows = jdbcTemplate.queryForObject("SELECT COUNT(*) FROM product_metrics", Integer.class); | ||
| System.out.printf("%n[시드 완료] 상품 %,d건 (%,dms) / 메트릭 %,d건 (%,dms)%n", | ||
| productCount, productSeedMs, metricRows, metricSeedMs); | ||
|
|
||
| // ── Weekly ── | ||
| t0 = System.currentTimeMillis(); | ||
| BatchStatus weeklyStatus = runJob("weekly"); | ||
| long weeklyMs = System.currentTimeMillis() - t0; | ||
|
|
||
| assertThat(weeklyStatus).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| int weeklyMvCount = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(weeklyMvCount).isEqualTo(100); | ||
|
|
||
| Long weeklyTopId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_weekly WHERE period_key = ? AND ranking = 1", | ||
| Long.class, TARGET_DATE); | ||
| // 급상승 그룹(1~5000) 중 p=5000이 최고 메트릭 | ||
| assertThat(weeklyTopId).isEqualTo(5_000L); | ||
|
|
||
| // ── Monthly ── | ||
| t0 = System.currentTimeMillis(); | ||
| BatchStatus monthlyStatus = runJob("monthly"); | ||
| long monthlyMs = System.currentTimeMillis() - t0; | ||
|
|
||
| assertThat(monthlyStatus).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| int monthlyMvCount = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_monthly WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(monthlyMvCount).isEqualTo(100); | ||
|
|
||
| Long monthlyTopId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_monthly WHERE period_key = ? AND ranking = 1", | ||
| Long.class, TARGET_DATE); | ||
| // 장기강자 그룹(5001~15000) 중 p=15000이 최고 메트릭 | ||
| assertThat(monthlyTopId).isEqualTo(15_000L); | ||
|
|
||
| // ── 파티션 균등 분배 검증 (monthly 실행 후 staging 기준) ── | ||
| int stagingTotal = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_staging WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(stagingTotal).isEqualTo(productCount); | ||
|
|
||
| // ── 결과 출력 ── | ||
| System.out.println(); | ||
| System.out.println("═══════════════════════════════════════════════════"); | ||
| System.out.println(" 대규모 배치 테스트 결과 (10만 건)"); | ||
| System.out.println("═══════════════════════════════════════════════════"); | ||
| System.out.printf(" 상품 수 : %,d%n", productCount); | ||
| System.out.printf(" 메트릭 행 수 : %,d%n", metricRows); | ||
| System.out.printf(" Partitioning : %d Worker%n", 4); | ||
| System.out.printf(" Weekly 소요 : %,dms (1위: product_%d, 급상승)%n", weeklyMs, weeklyTopId); | ||
| System.out.printf(" Monthly 소요 : %,dms (1위: product_%d, 장기강자)%n", monthlyMs, monthlyTopId); | ||
| System.out.printf(" Staging 적재 : %,d건 (~%,d건/partition)%n", stagingTotal, stagingTotal / 4); | ||
| System.out.println("═══════════════════════════════════════════════════"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MySQL Connector/J rewriteBatchedStatements batch insert performance
💡 Result:
The MySQL Connector/J property rewriteBatchedStatements=true significantly improves batch insert performance by rewriting batched INSERT (and REPLACE) statements into multi-value INSERT statements (e.g., INSERT INTO table VALUES (...),(...),(...)), reducing network round-trips and server parsing overhead. This works for both plain Statements and PreparedStatements (corrected documentation in 8.0.30+). It respects max_allowed_packet, splitting large batches automatically. Enable it in the JDBC URL: jdbc:mysql://host/db?rewriteBatchedStatements=true or via properties. Example: String url = "jdbc:mysql://localhost:3306/test?rewriteBatchedStatements=true"; Connection conn = DriverManager.getConnection(url, "user", "pass"); PreparedStatement ps = conn.prepareStatement("INSERT INTO test(id, name) VALUES (?, ?)"); for(int i=0; i<batchSize; i++) { ps.setInt(1, i); ps.setString(2, "name" + i); ps.addBatch; } ps.executeBatch; Performance gains: Up to 10x+ faster for large batches (e.g., Vlad Mihalcea benchmarks show mean batch time dropping from ~1072ms to ~112ms for 5000 inserts in batches of 100). StackOverflow tests confirm multi-row INSERTs reduce packets sent. Caveats (from official docs, since 3.1.13): - Risk of SQL injection with plain Statements if inputs unsanitized (use PreparedStatements). - For streams (set*Stream), specify length or risk "packet too large" errors. - getGeneratedKeys only for pure INSERT/REPLACE batches. - INSERT ... ON DUPLICATE KEY UPDATE returns aggregated counts (0 or SUCCESS_NO_INFO per statement). - Minimum 5 statements in batch for rewriting. - Not for mixed statements or non-INSERT/REPLACE. Active in latest Connector/J 9.0+ (2024) and 8.x LTS. Combine with continueBatchOnError=true for robustness.
Citations:
- 1: https://stackoverflow.com/questions/26307760/mysql-and-jdbc-with-rewritebatchedstatements-true
- 2: https://stackoverflow.com/questions/26307760/mysql-and-jdbc-with-rewritebatchedstatements-true/26313288
- 3: https://stackoverflow.com/questions/11389449/performance-of-mysql-insert-statements-in-java-batch-mode-prepared-statements-v
- 4: https://dev.mysql.com/doc/connectors/en/connector-j-connp-props-performance-extensions.html
- 5: https://vladmihalcea.com/mysql-rewritebatchedstatements
- 6: https://github.com/mysql/mysql-connector-j/blob/release/8.0/CHANGES
- 7: https://dev.mysql.com/doc/connector-j/en/connector-j-connp-props-performance-extensions.html
- 8: https://dev.mysql.com/doc/connector-j/en/connector-j-reference-implementation-notes.html
🏁 Script executed:
# First, locate the test file
find . -name "ProductRankingMvJobE2ETest.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 178
🏁 Script executed:
# Check the test class structure and annotations
head -100 ./apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4445
🏁 Script executed:
# Look for seedMetricsBulkWithTrends method implementation
grep -A 50 "seedMetricsBulkWithTrends" ./apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3049
🏁 Script executed:
# Check for datasource configuration in test resources
find ./apps/commerce-batch/src/test -name "*.properties" -o -name "*.yml" -o -name "*.yaml" | xargs grep -l "datasource\|rewriteBatchedStatements" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 69
🏁 Script executed:
# Search for test configuration classes
find ./apps/commerce-batch/src/test -name "*Configuration.java" -o -name "*Config.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Search for datasource configuration files in test resources
find ./apps/commerce-batch/src/test/resources -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \)Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check application-test or default properties
cat ./apps/commerce-batch/src/test/resources/application-test.yml 2>/dev/null || cat ./apps/commerce-batch/src/test/resources/application.yml 2>/dev/null || cat ./apps/commerce-batch/src/test/resources/application.properties 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Search for rewriteBatchedStatements in the entire codebase
rg "rewriteBatchedStatements" ./apps/commerce-batch/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check for any CI configuration that gates this test
find . -name ".github" -o -name "Jenkinsfile" -o -name ".gitlab-ci.yml" | xargs grep -l "ProductRankingMvJobE2ETest\|largeScalePartitionedBatchTest" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 69
🏁 Script executed:
# Look at the full seedProductsBulk to see batch implementation
grep -A 30 "private void seedProductsBulk" ./apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1422
대규모 E2E 테스트의 CI 파이프라인 격리 및 배치 성능 최적화가 필요하다.
이 테스트는 100만 행의 데이터를 시드하고 weekly/monthly 배치를 순차 실행하는데, 현재 어떤 제약 장치도 없어 모든 PR마다 CI에서 실행된다. 다음 두 가지 개선이 필수다:
-
테스트 격리:
@Tag("slow")및@Timeout을 부착하여 기본 파이프라인과 분리하거나, 퍼포먼스 전용 환경에서만 실행되도록@EnabledIfEnvironmentVariable("PERF_TEST_ENABLED", "true")추가. 대규모 배치는 Testcontainers MySQL 환경에서 2~5분 이상 소요되며, 자원 경합 시 간헐적 실패(플래키) 발생 위험이 높다. -
배치 삽입 성능:
seedProductsBulk와seedMetricsBulkWithTrends에서 사용하는jdbcTemplate.batchUpdate는 현재 MySQL JDBC 드라이버의rewriteBatchedStatements=true설정이 없으면, 각 배치마다 개별 네트워크 왕복이 발생한다. 100K 상품 × 30일 = 약 3,000회 왕복 발생. 테스트 데이터소스 설정에jdbc:mysql://localhost:3306/...?rewriteBatchedStatements=true를 추가하면 배치문을 다중값 INSERT로 재작성하여 10배 이상 성능 개선(수십 초 단축).
수정안:
- 테스트 클래스에
@Tag("slow")및@Timeout(minutes = 10)추가 - 테스트 계층 또는 CI 설정에서 느린 테스트 선택적 실행 구성
application-test.yml또는@TestConfiguration에서 테스트 데이터소스rewriteBatchedStatements=true활성화- 성능 개선 후 실제 실행 시간 측정 및
@Timeout값 조정
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`
around lines 561 - 634, The E2E test
ProductRankingMvJobE2ETest.largeScalePartitionedBatchTest is too heavy for
normal CI and its batch inserts (seedProductsBulk, seedMetricsBulkWithTrends)
suffer from JDBC batch rewrite missing; mark the test as isolated by adding
`@Tag`("slow") and `@Timeout`(minutes = 10) (or guard with
`@EnabledIfEnvironmentVariable`("PERF_TEST_ENABLED","true")) on the class or
method, and enable rewriteBatchedStatements=true for the test datasource (via
application-test.yml or a `@TestConfiguration` that configures the DataSource used
by jdbcTemplate) so jdbcTemplate.batchUpdate will be rewritten into multi-value
INSERTs; after applying these changes run the test once to adjust the timeout
value as needed.
Comment on lines
+638
to
+664
| @Test | ||
| @DisplayName("엣지 — 취소 반영: cancel_amount가 score에 반영") | ||
| void cancellationReflectedInScore() throws Exception { | ||
| seedProducts(2); | ||
|
|
||
| // 상품 1: 매출 100만, 취소 없음 | ||
| jdbcTemplate.update( | ||
| "INSERT INTO product_metrics (product_id, metric_date, view_count, like_count, unlike_count, " + | ||
| "sales_count, sales_amount, cancel_count_by_event_date, cancel_amount_by_event_date, " + | ||
| "cancel_count_by_order_date, cancel_amount_by_order_date) VALUES (1, '2026-04-16', 100, 10, 0, 10, 1000000, 0, 0, 0, 0)"); | ||
|
|
||
| // 상품 2: 매출 200만, 취소 150만 → 순 매출 50만 | ||
| jdbcTemplate.update( | ||
| "INSERT INTO product_metrics (product_id, metric_date, view_count, like_count, unlike_count, " + | ||
| "sales_count, sales_amount, cancel_count_by_event_date, cancel_amount_by_event_date, " + | ||
| "cancel_count_by_order_date, cancel_amount_by_order_date) VALUES (2, '2026-04-16', 200, 20, 0, 20, 2000000, 5, 1500000, 5, 1500000)"); | ||
|
|
||
| BatchStatus status = runJob("weekly"); | ||
|
|
||
| assertThat(status).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| // 상품 1이 1위 (순 매출 100만 > 상품 2 순 매출 50만) | ||
| Long topProductId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_weekly WHERE period_key = ? AND ranking = 1", | ||
| Long.class, TARGET_DATE); | ||
| assertThat(topProductId).isEqualTo(1L); | ||
| } |
There was a problem hiding this comment.
취소 반영 테스트는 metric_date='2026-04-16' 하드코딩으로 TARGET_DATE 변경 시 동기화가 깨진다.
TARGET_DATE = "20260416"이 변경되면 INSERT 문의 '2026-04-16'과 괴리가 생겨 weekly 윈도(최근 7일) 밖으로 떨어지고, 이 테스트는 assertion 없이 조용히 오답을 낼 수 있다(빈 MV). TARGET_DATE와 ISO 형식의 날짜 문자열을 단일 소스에서 파생시키는 것이 안전하다.
🔧 제안 수정
+ String metricDate = LocalDate.parse(TARGET_DATE, DATE_FORMATTER).toString(); // "2026-04-16"
- jdbcTemplate.update(
- "INSERT INTO product_metrics (...) VALUES (1, '2026-04-16', 100, 10, 0, 10, 1000000, 0, 0, 0, 0)");
+ jdbcTemplate.update(
+ "INSERT INTO product_metrics (product_id, metric_date, view_count, like_count, unlike_count, " +
+ "sales_count, sales_amount, cancel_count_by_event_date, cancel_amount_by_event_date, " +
+ "cancel_count_by_order_date, cancel_amount_by_order_date) " +
+ "VALUES (1, ?, 100, 10, 0, 10, 1000000, 0, 0, 0, 0)", metricDate);상품 2 쪽도 동일하게 적용한다.
추가 테스트로 "취소 금액이 매출 전액 이상일 때" 경계값 케이스(음수 순매출이 GREATEST(..., 0)으로 0으로 처리되는지)를 검증하는 케이스를 권한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`
around lines 638 - 664, The test cancellationReflectedInScore in
ProductRankingMvJobE2ETest hardcodes '2026-04-16', breaking sync with
TARGET_DATE; change the two INSERTs to derive the ISO date string from
TARGET_DATE (e.g., parse TARGET_DATE (yyyyMMdd) to a LocalDate and format as
yyyy-MM-dd) so the metric_date used in jdbcTemplate.update matches
TARGET_DATE-driven windows, update both product INSERT statements in
cancellationReflectedInScore (and any similar tests), and optionally add a new
edge test verifying when cancel_amount >= sales_amount the net sales floor to 0
(GREATEST(..., 0)).
Comment on lines
+119
to
+145
| CREATE TABLE IF NOT EXISTS mv_product_rank_weekly ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| product_id BIGINT NOT NULL, | ||
| ranking INT NOT NULL, | ||
| score DOUBLE NOT NULL, | ||
| view_count BIGINT NOT NULL DEFAULT 0, | ||
| like_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_amount BIGINT NOT NULL DEFAULT 0, | ||
| period_key VARCHAR(8) NOT NULL, | ||
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| INDEX idx_period_ranking (period_key, ranking) | ||
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS mv_product_rank_monthly ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| product_id BIGINT NOT NULL, | ||
| ranking INT NOT NULL, | ||
| score DOUBLE NOT NULL, | ||
| view_count BIGINT NOT NULL DEFAULT 0, | ||
| like_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_amount BIGINT NOT NULL DEFAULT 0, | ||
| period_key VARCHAR(8) NOT NULL, | ||
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| INDEX idx_period_ranking (period_key, ranking) | ||
| ); |
There was a problem hiding this comment.
MV 테이블에 (period_key, product_id) UNIQUE 제약 추가를 검토해야 한다.
운영 관점에서 mv_product_rank_weekly / mv_product_rank_monthly는 (product_id, period_key) 조합이 논리적으로 유일해야 하나, 현재 스키마는 id 외 어떤 UNIQUE 제약도 없다. CleanupTasklet의 DELETE와 mergeStep의 INSERT가 같은 트랜잭션에 묶여 있지 않은 상황에서 재시도/부분 실패가 발생하면 동일 period_key에 대해 동일 product가 2건 이상 누적될 수 있으며, findByPeriodKeyOrderByRankingAsc 조회가 중복 랭킹을 반환하여 페이지네이션이 깨진다. 운영/배치 단에서 가드가 있다 하더라도 스키마 레벨의 방어선은 유지해야 한다.
추가 테스트로는 "cleanupStep 성공 후 mergeStep을 인위적으로 2회 실행(혹은 cleanup skip 상태에서 merge 재실행)" 시나리오를 두어, UNIQUE 제약으로 INSERT가 실패하고 운영 지표에서 감지 가능한지 확인하는 것을 권한다.
🔒 제안 변경
CREATE TABLE IF NOT EXISTS mv_product_rank_weekly (
id BIGINT AUTO_INCREMENT PRIMARY KEY,
product_id BIGINT NOT NULL,
ranking INT NOT NULL,
score DOUBLE NOT NULL,
...
period_key VARCHAR(8) NOT NULL,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
- INDEX idx_period_ranking (period_key, ranking)
+ INDEX idx_period_ranking (period_key, ranking),
+ UNIQUE KEY uk_period_product (period_key, product_id)
);(월간 테이블도 동일하게 적용한다. 운영 DDL과 테스트 DDL이 함께 변경되어야 일치된다.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CREATE TABLE IF NOT EXISTS mv_product_rank_weekly ( | |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | |
| product_id BIGINT NOT NULL, | |
| ranking INT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| INDEX idx_period_ranking (period_key, ranking) | |
| ); | |
| CREATE TABLE IF NOT EXISTS mv_product_rank_monthly ( | |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | |
| product_id BIGINT NOT NULL, | |
| ranking INT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| INDEX idx_period_ranking (period_key, ranking) | |
| ); | |
| CREATE TABLE IF NOT EXISTS mv_product_rank_weekly ( | |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | |
| product_id BIGINT NOT NULL, | |
| ranking INT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| INDEX idx_period_ranking (period_key, ranking), | |
| UNIQUE KEY uk_period_product (period_key, product_id) | |
| ); | |
| CREATE TABLE IF NOT EXISTS mv_product_rank_monthly ( | |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | |
| product_id BIGINT NOT NULL, | |
| ranking INT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| INDEX idx_period_ranking (period_key, ranking), | |
| UNIQUE KEY uk_period_product (period_key, product_id) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-batch/src/test/resources/schema-batch-test.sql` around lines
119 - 145, Add a schema-level UNIQUE constraint on (period_key, product_id) for
both mv_product_rank_weekly and mv_product_rank_monthly to prevent duplicate
product rows per period; update the DDL in this test file (and corresponding
production DDL) to include UNIQUE(period_key, product_id) (or a named
constraint/index) and run tests that simulate cleanupStep success followed by
two mergeStep executions (or merge retry with cleanup skipped) to ensure
duplicate inserts fail and are detected; reference the tables
mv_product_rank_weekly, mv_product_rank_monthly, the CleanupTasklet, mergeStep,
and the findByPeriodKeyOrderByRankingAsc query when adding the constraint and
test.
Comment on lines
+147
to
+156
| CREATE TABLE IF NOT EXISTS mv_product_rank_staging ( | ||
| product_id BIGINT NOT NULL, | ||
| score DOUBLE NOT NULL, | ||
| view_count BIGINT NOT NULL DEFAULT 0, | ||
| like_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_count BIGINT NOT NULL DEFAULT 0, | ||
| sales_amount BIGINT NOT NULL DEFAULT 0, | ||
| period_key VARCHAR(8) NOT NULL, | ||
| PRIMARY KEY (product_id, period_key) | ||
| ); |
There was a problem hiding this comment.
mv_product_rank_staging의 period_key 컬럼에 인덱스를 추가해야 한다.
현재 PK는 (product_id, period_key)로 선두가 product_id이므로, CleanupTasklet의 DELETE FROM mv_product_rank_staging WHERE period_key = ? 및 DELETE ... WHERE period_key < ?, 그리고 mergeStep의 SELECT ... WHERE period_key = ? ORDER BY score DESC LIMIT 100 모두 인덱스를 타지 못하고 풀스캔으로 동작한다. 대규모(10만×30일) 환경에서 staging 정리/집계 지연이 누적되는 문제로 이어질 수 있다.
추가 테스트로는 largeScalePartitionedBatchTest에서 EXPLAIN 결과를 통해 type=range 혹은 ref 여부를 점검하거나, cleanup/merge 단계별 소요 시간을 로그로 측정하여 회귀 여부를 관찰하는 것이 바람직하다.
🔧 제안 변경
CREATE TABLE IF NOT EXISTS mv_product_rank_staging (
product_id BIGINT NOT NULL,
...
period_key VARCHAR(8) NOT NULL,
- PRIMARY KEY (product_id, period_key)
+ PRIMARY KEY (product_id, period_key),
+ INDEX idx_staging_period (period_key)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CREATE TABLE IF NOT EXISTS mv_product_rank_staging ( | |
| product_id BIGINT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| PRIMARY KEY (product_id, period_key) | |
| ); | |
| CREATE TABLE IF NOT EXISTS mv_product_rank_staging ( | |
| product_id BIGINT NOT NULL, | |
| score DOUBLE NOT NULL, | |
| view_count BIGINT NOT NULL DEFAULT 0, | |
| like_count BIGINT NOT NULL DEFAULT 0, | |
| sales_count BIGINT NOT NULL DEFAULT 0, | |
| sales_amount BIGINT NOT NULL DEFAULT 0, | |
| period_key VARCHAR(8) NOT NULL, | |
| PRIMARY KEY (product_id, period_key), | |
| INDEX idx_staging_period (period_key) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-batch/src/test/resources/schema-batch-test.sql` around lines
147 - 156, The mv_product_rank_staging table's primary key is (product_id,
period_key) which prevents range scans on period_key alone and causes full table
scans in CleanupTasklet (DELETE ... WHERE period_key = ?) and mergeStep (SELECT
... WHERE period_key = ? ORDER BY score DESC LIMIT 100); add a secondary index
on period_key (e.g., CREATE INDEX ON mv_product_rank_staging(period_key)) to
ensure those queries use an index. After adding the index, validate in
largeScalePartitionedBatchTest by running EXPLAIN for the cleanup and merge
queries to confirm index usage (type=range or ref) and measure cleanup/merge
step durations to detect regressions.
- Summary: 배경/목표/결과 구조로 변경 - Context & Decision: 문제 정의 + 선택지 5개(대안→결정→트레이드오프) - Design Overview: 변경 범위, 컴포넌트 책임 - Flow Diagram: Mermaid 2개 (배치 Job + API 조회) - 리뷰 포인트 내용 갱신
- 선택지 순서: Chunk/Tasklet → Reader/병렬 → fallback → 재계산 → Score - 지수 감쇠의 전시 기간 희석 특징 추가 - 문제 정의에서 Redis 언급 제거 (주간/월간 집계는 이번에 신규)
…s), 2.1x 향상 - GRID_SIZE를 @value로 외부 주입 가능하게 변경 (ReflectionTestUtils로 테스트 내 동적 변경) - partitionBenchmark 테스트 추가: 동일 데이터(10만×30일)에서 gridSize만 교체하여 측정 - PR draft, batch-test-results, blog 문서에 벤치마크 결과 반영 (10/10 PASSED)
- 성능 테이블을 규모별 비교표(150 / 1,020 / 100,000)로 교체 - 섹션 6에 10만 건 테스트의 1위 검증 결과 추가 - 1,020개 데이터는 실환경 API 검증 맥락으로 유지
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java (1)
116-129: Partitioner의SELECT DISTINCT product_id전량 적재는 대용량 구간에서 드라이버 메모리·디스패치 지연의 병목이 될 수 있다.현재는
product_metrics의 30일 구간 전체에 대해 distinct product_id를 정렬·적재한 뒤 드라이버 JVM 메모리에서 슬라이싱한다. 100K 규모에서는 문제 없으나 수백만 단위에서는 distinct 소트가idx_metric_date만으로 커버되지 않아 temp 정렬이 발생하고, 파티셔너가 Step 디스패치 전에 동기 블로킹된다. 운영상 수평 확장성이 제한된다.대안으로 (1)
MIN/MAX(product_id)+ 균등 범위 분할로 ID 리스트 적재를 회피하거나, (2)product_metrics(metric_date, product_id)복합 인덱스로 loose index scan을 유도해 distinct 비용을 낮추는 방향을 권장한다. (2)는 운영 마이그레이션이 필요하므로 배포 계획에 포함되어야 한다.추가 테스트로 100만 상품 시드 시 파티셔너 단독 수행 시간을 측정하여 임계값(예: 500ms)을 넘기면 실패시키는 회귀 테스트를 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java` around lines 116 - 129, The current partitioner in ProductRankingMvJobConfig loads all distinct product_id via jdbcTemplate.queryForList into productIds which can OOM/block; change it to avoid materializing the full ID list by querying MIN and MAX product_id (use jdbcTemplate.queryForObject with "SELECT MIN(product_id), MAX(product_id) ...") and compute equal numeric ranges to create partitions (populate ExecutionContext minProductId/maxProductId per partition) instead of slicing a productIds list; alternatively, if opting for DB-side improvement, add a note to create a composite index on (metric_date, product_id) to enable a loose index scan; keep the existing ExecutionContext keys ("minProductId","maxProductId") and partition naming (e.g., "partitionN") and add a regression test that runs the partitioner solo against a 1M-product seed and asserts the partitioning latency under the target threshold.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`:
- Around line 640-694: partitionBenchmark creates heavy CI load, leaks mutable
configuration and lacks assertions; change it to be gated as a slow test (e.g.,
add `@Tag`("slow") or skip when an env var like RUN_SLOW_TESTS is not set), ensure
jobConfig.gridSize is restored after modification (wrap
ReflectionTestUtils.setField(jobConfig, "gridSize", ...) uses in a try/finally
or restore in an `@AfterEach`), guard the speedup calculation by asserting
partitionedMs > 0 and adding a numeric assertion (e.g., speedup > 1.0 with a
sensible tolerance) instead of just printing, and add a deterministic
result-equality check (after running runJob("weekly") for gridSize=1 and
gridSize=4 compare the MV rows for product_id, ranking, score from
mv_product_rank_weekly or write a separate test that seeds a smaller dataset and
verifies runJob produces identical MV contents) so partitioning doesn’t change
results.
---
Nitpick comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.java`:
- Around line 116-129: The current partitioner in ProductRankingMvJobConfig
loads all distinct product_id via jdbcTemplate.queryForList into productIds
which can OOM/block; change it to avoid materializing the full ID list by
querying MIN and MAX product_id (use jdbcTemplate.queryForObject with "SELECT
MIN(product_id), MAX(product_id) ...") and compute equal numeric ranges to
create partitions (populate ExecutionContext minProductId/maxProductId per
partition) instead of slicing a productIds list; alternatively, if opting for
DB-side improvement, add a note to create a composite index on (metric_date,
product_id) to enable a loose index scan; keep the existing ExecutionContext
keys ("minProductId","maxProductId") and partition naming (e.g., "partitionN")
and add a regression test that runs the partitioner solo against a 1M-product
seed and asserts the partitioning latency under the target threshold.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3393850-cc86-4fd0-820f-1b629fd0fe28
⛔ Files ignored due to path filters (5)
docs/design/volume-10/10-batch-test-results.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-partition-benchmark-prompt.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-pr-draft.mdis excluded by!**/*.mdand included by**docs/design/volume-10/10-technical-writing-plan.mdis excluded by!**/*.mdand included by**docs/design/volume-10/11-ranking-batch-test-blog.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (2)
apps/commerce-batch/src/main/java/com/loopers/batch/job/rankingmv/ProductRankingMvJobConfig.javaapps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java
Comment on lines
+640
to
+694
| @Test | ||
| @DisplayName("벤치마크 — gridSize=1 vs gridSize=4 소요 시간 비교") | ||
| void partitionBenchmark() throws Exception { | ||
| int productCount = 100_000; | ||
| int metricDays = 30; | ||
|
|
||
| long t0 = System.currentTimeMillis(); | ||
| seedProductsBulk(productCount); | ||
| seedMetricsBulkWithTrends(productCount, metricDays, TARGET_DATE); | ||
| long seedMs = System.currentTimeMillis() - t0; | ||
|
|
||
| int metricRows = jdbcTemplate.queryForObject("SELECT COUNT(*) FROM product_metrics", Integer.class); | ||
| System.out.printf("%n[시드 완료] 상품 %,d건, 메트릭 %,d건 (%,dms)%n", productCount, metricRows, seedMs); | ||
|
|
||
| // ── gridSize=1 (단일 스레드) ── | ||
| ReflectionTestUtils.setField(jobConfig, "gridSize", 1); | ||
|
|
||
| t0 = System.currentTimeMillis(); | ||
| BatchStatus singleStatus = runJob("weekly"); | ||
| long singleMs = System.currentTimeMillis() - t0; | ||
| assertThat(singleStatus).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| int singleMvCount = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(singleMvCount).isEqualTo(100); | ||
|
|
||
| // ── 중간 정리 ── | ||
| jdbcTemplate.update("DELETE FROM mv_product_rank_weekly WHERE period_key = ?", TARGET_DATE); | ||
| jdbcTemplate.update("DELETE FROM mv_product_rank_staging WHERE period_key = ?", TARGET_DATE); | ||
|
|
||
| // ── gridSize=4 (4 Partition 병렬) ── | ||
| ReflectionTestUtils.setField(jobConfig, "gridSize", 4); | ||
|
|
||
| t0 = System.currentTimeMillis(); | ||
| BatchStatus partitionedStatus = runJob("weekly"); | ||
| long partitionedMs = System.currentTimeMillis() - t0; | ||
| assertThat(partitionedStatus).isEqualTo(BatchStatus.COMPLETED); | ||
|
|
||
| int partitionedMvCount = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE period_key = ?", | ||
| Integer.class, TARGET_DATE); | ||
| assertThat(partitionedMvCount).isEqualTo(100); | ||
|
|
||
| double speedup = (double) singleMs / partitionedMs; | ||
|
|
||
| System.out.println(); | ||
| System.out.println("═══════════════════════════════════════"); | ||
| System.out.println(" Partitioning 벤치마크 (10만 상품)"); | ||
| System.out.println("═══════════════════════════════════════"); | ||
| System.out.printf(" gridSize=1: %,dms%n", singleMs); | ||
| System.out.printf(" gridSize=4: %,dms%n", partitionedMs); | ||
| System.out.printf(" 향상률: %.1fx%n", speedup); | ||
| System.out.println("═══════════════════════════════════════"); | ||
| } |
There was a problem hiding this comment.
partitionBenchmark는 CI 격리·상태 복구·수치 검증 모두 부재하여 플래키·오염 위험이 크다.
운영 관점에서 세 가지 문제가 동시에 존재한다.
- CI 부담:
largeScalePartitionedBatchTest와 동일하게 100K × 30일(3M 행)을 시드한 뒤 weekly Job을 2회 실행한다. 기본 파이프라인에서 매 PR 실행될 경우 자원 경합과 타임아웃 위험이 누적된다.@Tag("slow")또는 환경변수 gating이 필요하다. - 싱글톤 상태 누수:
ReflectionTestUtils.setField(jobConfig, "gridSize", ...)로@Configuration빈의 필드를 직접 변경한 뒤@AfterEach에서 원복하지 않는다. 테스트 실행 순서에 따라gridSize=1상태가 후속 테스트로 전파될 수 있으며, 현재는 마지막에 4로 되돌려 기본값과 우연히 일치할 뿐이다. 테스트 격리 원칙 위반이다. - 검증 부재 + 0 나눗셈:
(double) singleMs / partitionedMs는partitionedMs == 0이면Infinity를 출력하고, speedup에 대한 assertion이 없어 회귀 시에도 테스트가 통과한다. 벤치마크의 목적이 "파티셔닝이 실제로 빠르다"의 보증이라면 수치 검증이 필요하다.
제안 수정 — 격리·복구·검증 보강
+ `@org.junit.jupiter.api.Tag`("slow")
+ `@org.junit.jupiter.api.Timeout`(value = 10, unit = java.util.concurrent.TimeUnit.MINUTES)
`@Test`
`@DisplayName`("벤치마크 — gridSize=1 vs gridSize=4 소요 시간 비교")
void partitionBenchmark() throws Exception {
int productCount = 100_000;
int metricDays = 30;
+ int originalGridSize = (int) ReflectionTestUtils.getField(jobConfig, "gridSize");
+ try {
...
- double speedup = (double) singleMs / partitionedMs;
+ double speedup = partitionedMs == 0 ? Double.POSITIVE_INFINITY
+ : (double) singleMs / partitionedMs;
+ assertThat(partitionedMs).isLessThan(singleMs);
...
+ } finally {
+ ReflectionTestUtils.setField(jobConfig, "gridSize", originalGridSize);
+ }
}추가 테스트로 gridSize=1로 단독 돌렸을 때와 gridSize=4로 단독 돌렸을 때 결과 MV(product_id, ranking, score)가 동일한지를 검증하는 케이스를 권한다(파티셔닝이 결과에 영향 주지 않음을 보장).
As per coding guidelines ("통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/rankingmv/ProductRankingMvJobE2ETest.java`
around lines 640 - 694, partitionBenchmark creates heavy CI load, leaks mutable
configuration and lacks assertions; change it to be gated as a slow test (e.g.,
add `@Tag`("slow") or skip when an env var like RUN_SLOW_TESTS is not set), ensure
jobConfig.gridSize is restored after modification (wrap
ReflectionTestUtils.setField(jobConfig, "gridSize", ...) uses in a try/finally
or restore in an `@AfterEach`), guard the speedup calculation by asserting
partitionedMs > 0 and adding a numeric assertion (e.g., speedup > 1.0 with a
sensible tolerance) instead of just printing, and add a deterministic
result-equality check (after running runJob("weekly") for gridSize=1 and
gridSize=4 compare the MV rows for product_id, ranking, score from
mv_product_rank_weekly or write a separate test that seeds a smaller dataset and
verifies runJob produces identical MV contents) so partitioning doesn’t change
results.
SukheeChoi
changed the title
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
1 participant
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.
📌 Summary
product_metrics(일간 메트릭)를 주간/월간 단위로 합산하여 MV 테이블에 TOP 100 랭킹을 적재하고, API에서 조회할 수 있도록 한다.🧭 Context & Decision
문제 정의
product_metrics)은 적재되어 있지만, 주간/월간 단위의 기간 집계 랭킹은 존재하지 않는다.product_metrics기반으로 주간(7일)/월간(30일) 메트릭을 합산하여 TOP 100 랭킹을 MV 테이블에 적재하고, API에서 조회할 수 있다.선택지와 결정
1. Score 계산 방식
0.97^i를 곱하여 오래된 날일수록 가중치를 줄임(반감기 약 23일). 같은 총 매출이라도 최근에 집중된 상품이 더 높은 순위를 받음. 전시 기간이 길어서 누적된 score가 높은 상품의 이점을 희석할 수 있다는 특징이 있음2. 전체 재계산 vs 증분 계산
cancel_by_order_date가 과거 행을 사후 갱신하므로 이 전제가 깨진다. 성능 차이(~10초 vs ~3초)는 1일 1회 배치에서 운영 영향 없음3. Chunk vs Tasklet
INSERT INTO...SELECT + RANK() OVER + LIMIT 100으로 SQL 한 방 처리. 네트워크 왕복 0faultTolerant + retry + ExponentialBackOffPolicy,StepExecution자동 기록,StepMonitorListener)을 활용할 수 있다. 100건에 대한 네트워크 왕복 비용(< 1ms)보다 이 운영 기능의 가치가 크다4. Reader 선택 + 병렬 처리
5. Redis fallback vs 전일 MV fallback
🏗️ Design Overview
변경 범위
modules/jpa,commerce-batch,commerce-streamer,commerce-apiScoreFormula.java(modules/jpa) — Score 공식 Single Source of Truth. 3개 앱 모듈이 공유ScoreFormulaTest.java(modules/jpa) — Score 공식 단위 테스트ProductRankingMvJobConfig.java— Job + 3 Step + Partitioner + Reader + Processor + WriterCleanupTasklet.java— DELETE + 데이터 보존 정책MvProductRank.java/MvProductRankWeekly.java/MvProductRankMonthly.java— MV 엔티티MvProductRankRepository.java+ JPA 구현체 — MV 조회mv_product_rank_weekly/mv_product_rank_monthly/mv_product_rank_staging— DDLRankingScoreUpdater.java— calculateScore()를 ScoreFormula에 위임, weekly/monthly 키 생성 메서드 및 상수 제거RankingCorrectionJobConfig.java— calculateScore()를 ScoreFormula에 위임RankingProperties.java/RankingCorrectionProperties.java— Weights inner record 제거, ScoreFormula.Weights 사용RankingFacade.java— weekly/monthly 조회 경로를 Redis → MV로 변경RankingCarryOverScheduler.java— Redis weekly/monthly carry-over 제거 (MV가 담당하므로 daily carry-over만 유지)RankingRedisRepository.java— 미사용 RANKING_WEEKLY_PREFIX/RANKING_MONTHLY_PREFIX 상수 제거주요 컴포넌트 책임
ProductRankingMvJobConfig: 3-Step Job 오케스트레이션. Partitioner로 product_id 범위 분할, Worker Step에서 Chunk-Oriented 집계, mergeStep에서 Global TOP 100 추출CleanupTasklet: 당일 period_key의 MV/staging DELETE + 3일 이전 데이터 퍼지. 멱등성 보장의 핵심RankingFacade: scope별 데이터 소스 분기. daily → Redis, weekly/monthly → MV(당일 → 전일 fallback)🔁 Flow Diagram
배치 Job 흐름
flowchart TD A[ProductRankingMvJob 시작] --> B[Step 1: CleanupTasklet] B -->|FAILED| Z[Job 종료] B -->|COMPLETED| C[Step 2: Partitioned Aggregate] C --> D1[Worker 1: product_id 1~25000] C --> D2[Worker 2: product_id 25001~50000] C --> D3[Worker 3: product_id 50001~75000] C --> D4[Worker 4: product_id 75001~100000] D1 -->|GROUP BY → ScoreFormula| S[staging 테이블] D2 -->|GROUP BY → ScoreFormula| S D3 -->|GROUP BY → ScoreFormula| S D4 -->|GROUP BY → ScoreFormula| S S --> E[Step 3: Merge] E -->|ROW_NUMBER + LIMIT 100| F[MV 테이블 TOP 100]API 조회 흐름
sequenceDiagram autonumber participant Client participant RankingFacade participant MvProductRankRepo participant RankingRedisRepo participant ProductRepo Client->>RankingFacade: GET /api/v1/rankings?scope=weekly alt scope = daily RankingFacade->>RankingRedisRepo: ZREVRANGE (Redis ZSET) RankingRedisRepo-->>RankingFacade: RankingEntry[] else scope = weekly | monthly RankingFacade->>MvProductRankRepo: findByPeriodKey(당일) MvProductRankRepo-->>RankingFacade: MvProductRank[] alt 당일 데이터 없음 RankingFacade->>MvProductRankRepo: findByPeriodKey(전일) MvProductRankRepo-->>RankingFacade: MvProductRank[] end end RankingFacade->>ProductRepo: findAllByIds(productIds) ProductRepo-->>RankingFacade: ProductWithBrand[] RankingFacade-->>Client: PagedRankingResponse테스트 결과
E2E 테스트: 10/10 PASSED
ProductRankingMvJobE2ETestPartitioning 벤치마크 (gridSize=1 vs gridSize=4)
동일 데이터(10만 상품 × 30일 = 300만 행)를
ReflectionTestUtils로 gridSize만 교체하여 weekly/monthly 각 2회 측정. 4 Partition 병렬이 단일 스레드 대비 weekly 2.1x, monthly 1.8x 빠르다.데이터 4배(70만→300만)에도 gridSize=4 기준 소요 시간은 25%만 증가(1,746ms→2,188ms). Reader SQL의 GROUP BY가 scope와 무관하게 결과를 10만 건으로 압축하므로, Processor/Writer/Merge가 데이터 볼륨에 영향받지 않는 구조.
리뷰 포인트
Partitioning + CursorReader 조합시에 적절한 gridSize, 스테이징 테이블을 두는 효용 산정 방식
요구사항에 "대량의 데이터를 읽고 처리할 수 있도록 구성"이 명시되어 있어, 활성 상품 수가 수십만~수백만 규모로 성장하더라도 배치 윈도우 내에 처리 가능한 구조를 고려했습니다.
GROUP BY 집계에서 PagingReader는 페이지마다 집계를 재실행하고, CursorReader는 멀티스레드에서 사용이 어려워서, Partitioning으로 product_id 범위를 분할하여 각 Worker가 독립 CursorReader를 갖도록 했습니다.
질문: