[5주차] 읽기 성능 최적화: 인덱스 설계 + 캐시 전략 - 최호석#212
Open
ghtjr410 wants to merge 5 commits intoLoopers-dev-lab:ghtjr410from
Open
[5주차] 읽기 성능 최적화: 인덱스 설계 + 캐시 전략 - 최호석#212ghtjr410 wants to merge 5 commits intoLoopers-dev-lab:ghtjr410from
ghtjr410 wants to merge 5 commits intoLoopers-dev-lab:ghtjr410from
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughOrder 상태 모델에 상태 전이 및 인덱스가 추가되고, 제품 조회에 Redis/Caffeine 기반 다층 캐시(L1/L2)와 Pub/Sub 무효화가 도입되었으며, V1/V2/V3 실험 API·컨트롤러·테스트·성능 시나리오가 추가되었다다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as ProductExperimentController
participant Facade as ProductExperimentFacade
participant L1 as ProductLocalCacheManager
participant L2 as ProductCacheManager
participant Service as ProductService
participant DB as Database
User->>Controller: GET /api/experiment/products/v3/{id}
Controller->>Facade: getDetailV3(productId, userId)
Facade->>L1: getDetail(productId)
alt L1 hit
L1-->>Facade: ProductInfo
else L1 miss
Facade->>L2: getDetail(productId)
alt L2 hit
L2-->>Facade: ProductInfo
Facade->>L1: putDetail(productId, info)
else L2 miss
Facade->>Service: findActiveProduct(productId)
Service->>DB: SELECT product by id
DB-->>Service: product row
Service-->>Facade: Product
Facade->>L2: putDetail(productId, info)
Facade->>L1: putDetail(productId, info)
end
end
Facade->>Service: isLiked(userId, productId)
Service->>DB: SELECT exists like
DB-->>Service: boolean
Service-->>Facade: liked
Facade-->>Controller: ProductExperimentInfo
Controller-->>User: ApiResponse<DetailResponse>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 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)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Pull request overview
상품/주문 조회의 읽기 성능을 개선하기 위해 DB 인덱스 설계와 상품 조회 캐시(Redis + 실험용 Caffeine L1), 그리고 이를 검증/재현하기 위한 실험 API·k6 스크립트·분석 문서를 추가/갱신한 PR입니다.
Changes:
- 상품/주문 조회 최적화를 위한 인덱스 추가 및 조회 경로 보강(상품 cursor 조회 포함)
- 상품 상세/목록 Redis 캐시 + 무효화(Pub/Sub, afterCommit) 및 실험용 L1(Caffeine) 계층 추가
- k6 부하 테스트 스크립트 및 시드/EXPLAIN 분석 문서 추가
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java | Redis 테스트컨테이너 설정 정리 |
| modules/redis/src/main/java/com/loopers/config/redis/RedisConfig.java | Pub/Sub 전용 Redis 커넥션 추가 |
| k6/list-test.js | 목록 성능 측정 k6 시나리오 |
| k6/detail-test.js | 상세 성능 측정 k6 시나리오 |
| docs/performance/product/seed-products.sql | 상품 10만 시드 생성 스크립트 |
| docs/performance/product/index-analysis.md | 상품 인덱스 분석 문서 |
| docs/performance/product/explain-analysis.sql | 상품 EXPLAIN 실행 SQL |
| docs/performance/product/cache-strategy.md | 상품 캐시 전략 문서 |
| docs/performance/order/seed-orders.sql | 주문/아이템 시드 생성 스크립트 |
| docs/performance/order/index-analysis.md | 주문 인덱스 분석 문서 |
| docs/performance/order/explain-analysis.sql | 주문 EXPLAIN 실행 SQL |
| apps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductCacheManagerIntegrationTest.java | Redis 캐시 매니저 통합 테스트 |
| apps/commerce-api/src/test/java/com/loopers/application/order/OrderServiceIntegrationTest.java | 주문 조회 API 변경 반영 테스트 수정 |
| apps/commerce-api/src/main/java/com/loopers/support/transaction/TransactionHelper.java | afterCommit 헬퍼 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.java | 주문 응답에 status 노출 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java | 사용자 주문 목록 status 필터 전달 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderRequest.java | 주문 목록 요청에 status/정렬 추가 + by-product 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Dto.java | 관리자 주문 응답에 status 노출 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.java | 관리자 status 필터 + by-product 엔드포인트 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminApiV1Spec.java | 관리자 by-product API 스펙 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentV1Dto.java | 상품 실험 API DTO 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentRequest.java | 상품 실험 API 요청 DTO 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentController.java | 상품 v1/v2/v3 실험 API 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentV1Dto.java | 주문 실험 API DTO 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentRequest.java | 주문 실험 API 요청 DTO 추가 |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/CursorResponse.java | 커서 응답 래퍼 추가 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java | 커서 조회/정리 배치용 repo 메서드 추가 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheManager.java | Caffeine 기반 L1 캐시 매니저 추가 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java | 정리용/커서 조회용 쿼리 추가 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.java | Redis 캐시(상세/목록) + 키 레지스트리 추가 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheInvalidationConfig.java | Redis Pub/Sub 기반 L1 무효화 구성 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderRepositoryImpl.java | 주문 조회 레포지토리 메서드 확장 |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderJpaRepository.java | status/user/product 기반 조회 쿼리 추가 |
| apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java | 정리 배치/커서 조회 인터페이스 변경 |
| apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java | products 인덱스 애노테이션 추가 |
| apps/commerce-api/src/main/java/com/loopers/domain/order/OrderStatus.java | 주문 상태 enum 추가 |
| apps/commerce-api/src/main/java/com/loopers/domain/order/OrderRepository.java | 주문 조회 인터페이스 확장 |
| apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.java | order_item 인덱스 애노테이션 추가 |
| apps/commerce-api/src/main/java/com/loopers/domain/order/Order.java | orders 인덱스 + status/상태전이 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/product/ProductService.java | 정리 배치/커서 조회 서비스 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/product/ProductScheduler.java | 배치 정리 로직 변경(IDs 기반) |
| apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java | 상품 상세/목록 Redis 캐시 적용 + afterCommit 무효화 |
| apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentInfo.java | 실험용 Product info 레코드 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.java | v1/v2/v3 실험 경로 구현 |
| apps/commerce-api/src/main/java/com/loopers/application/order/OrderService.java | 주문 조회 서비스 API 확장 |
| apps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.java | OrderInfo/요약 DTO에 status 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java | 사용자/관리자 목록 status 필터 + by-product 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/like/LikeService.java | liked 여부 조회 메서드 추가 |
| apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java | like/unlike 후 afterCommit 상세 캐시 무효화 |
| apps/commerce-api/build.gradle.kts | Caffeine 의존성 추가 |
| .claude/rules/project/architecture.md | 아키텍처 문구 수정 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| if (message.startsWith("detail:")) { | ||
| String productIdStr = message.substring("detail:".length()); | ||
| localCacheManager.evictDetail(Long.parseLong(productIdStr)); |
Comment on lines
+57
to
+62
| CASE | ||
| WHEN RAND() < 0.02 THEN DATE_SUB(NOW(), INTERVAL FLOOR(RAND() * 6) HOUR) -- PENDING: 최근 수시간 | ||
| WHEN RAND() < 0.05 THEN DATE_SUB(NOW(), INTERVAL FLOOR(RAND() * 3) DAY) -- PAID: 최근 2~3일 | ||
| WHEN RAND() < 0.10 THEN DATE_SUB(NOW(), INTERVAL FLOOR(RAND() * 14) DAY) -- SHIPPING: 최근 2주 | ||
| ELSE DATE_SUB(NOW(), INTERVAL FLOOR(14 + RAND() * 351) DAY) -- DELIVERED/CANCELLED: 2주~1년전 | ||
| END AS created_at |
Comment on lines
+89
to
+100
| public Optional<CachedPage> getList(Long brandId, String sort, int page, int size) { | ||
| String json = redisTemplate.opsForValue().get(listKey(brandId, sort, page, size)); | ||
| if (json == null) { | ||
| return Optional.empty(); | ||
| } | ||
| try { | ||
| return Optional.of(objectMapper.readValue(json, CachedPage.class)); | ||
| } catch (JsonProcessingException e) { | ||
| log.warn("상품 목록 캐시 역직렬화 실패. brandId={}, sort={}", brandId, sort, e); | ||
| return Optional.empty(); | ||
| } | ||
| } |
Comment on lines
+160
to
+175
| String cacheKey = cursorCacheKey(brandId, cursor, size); | ||
| Optional<CachedPage> cached = productCacheManager.getList(brandId, cacheKey, 0, size); | ||
| if (cached.isPresent()) { | ||
| CachedPage cp = cached.get(); | ||
| boolean hasNext = cp.totalElements() > 0; | ||
| Long nextCursor = hasNext && !cp.content().isEmpty() | ||
| ? cp.content().get(cp.content().size() - 1).id() | ||
| : null; | ||
| return new CursorResult(cp.content(), nextCursor, hasNext); | ||
| } | ||
|
|
||
| List<Product> products = productService.findActiveProductsCursor(brandId, cursor, size + 1); | ||
| CursorResult result = toCursorResult(products, size); | ||
| productCacheManager.putList(brandId, cacheKey, 0, size, | ||
| new CachedPage(result.content(), 0, size, result.hasNext() ? 1 : 0)); | ||
| return result; |
Comment on lines
+39
to
+43
| CASE | ||
| WHEN RAND() < 0.30 THEN FLOOR(RAND() * 100) + 1 -- 상위 1% (100명) → 30% 주문 | ||
| WHEN RAND() < 0.60 THEN FLOOR(RAND() * 900) + 101 -- 상위 10% (900명) → 30% 주문 | ||
| ELSE FLOOR(RAND() * 9000) + 1001 -- 나머지 (9,000명) → 40% 주문 | ||
| END AS user_id, |
Comment on lines
+45
to
+51
| CASE | ||
| WHEN RAND() < 0.02 THEN 'PENDING' | ||
| WHEN RAND() < 0.05 THEN 'PAID' | ||
| WHEN RAND() < 0.10 THEN 'SHIPPING' | ||
| WHEN RAND() < 0.35 THEN 'CANCELLED' | ||
| ELSE 'DELIVERED' | ||
| END AS status, |
Comment on lines
+1
to
+3
| import http from 'k6/http'; | ||
| import { check } from 'k6'; | ||
| import { Trend, Counter } from 'k6/metrics'; |
Comment on lines
+76
to
+86
| public Optional<ProductInfo> getDetail(Long productId) { | ||
| String json = redisTemplate.opsForValue().get(detailKey(productId)); | ||
| if (json == null) { | ||
| return Optional.empty(); | ||
| } | ||
| try { | ||
| return Optional.of(objectMapper.readValue(json, ProductInfo.class)); | ||
| } catch (JsonProcessingException e) { | ||
| log.warn("상품 상세 캐시 역직렬화 실패. productId={}", productId, e); | ||
| return Optional.empty(); | ||
| } |
Comment on lines
+41
to
+47
| public void putList(Long brandId, String sort, int page, int size, CachedPage cachedPage) { | ||
| try { | ||
| String key = listKey(brandId, sort, page, size); | ||
| String json = objectMapper.writeValueAsString(cachedPage); | ||
| redisTemplate.opsForValue().set(key, json, LIST_TTL); | ||
| redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key); | ||
| } catch (JsonProcessingException e) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentRequest.java (2)
55-74:⚠️ Potential issue | 🟠 MajorAdminStatusList도 동일하게 Bean Validation 어노테이션을 사용하고 있다.
위 UserOrderList 리뷰 내용과 동일한 사유로 어노테이션 제거 및 도메인 레이어 검증으로 전환이 필요하다.
🤖 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/interfaces/api/experiment/OrderExperimentRequest.java` around lines 55 - 74, AdminStatusList 레코드에서 Bean Validation(`@NotNull`, `@PositiveOrZero`, `@Min`, `@Max`)을 제거하고 도메인 레이어로 검증을 이동하세요: AdminStatusList의 필드 선언부에서 어노테이션들을 삭제하고, 생성자 블록(public AdminStatusList) 또는 호출 지점에서 OrderStatus null 체크 및 page/size 기본값 설정 로직은 유지하되 유효성(예: page >= 0, 1 <= size <= 100)을 도메인 서비스나 엔티티 유효성 검사 메서드로 옮겨 검증 예외를 발생시키도록 변경하세요; 또한 toPageable()는 그대로 두되 도메인 검증을 거친 값만 전달되도록 호출 흐름을 수정해 주세요.
76-95:⚠️ Potential issue | 🟠 MajorProductOrderList도 동일하게 Bean Validation 어노테이션을 사용하고 있다.
위 UserOrderList 리뷰 내용과 동일한 사유로 어노테이션 제거 및 도메인 레이어 검증으로 전환이 필요하다.
🤖 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/interfaces/api/experiment/OrderExperimentRequest.java` around lines 76 - 95, ProductOrderList currently uses Bean Validation annotations (`@NotNull`, `@PositiveOrZero`, `@Min`, `@Max`) on its record components; remove those annotations from the ProductOrderList record (productId, page, size) and rely on domain-layer validation instead — ensure the existing compact constructor still sets defaults for page and size, keep toPageable() as-is, and add/ensure validation logic in the domain/service conversion path that constructs the domain object (validate productId non-null, page >= 0, size between 1 and 100) so all invariant checks live in the domain layer rather than the API DTO.
🟡 Minor comments (11)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentRequest.java-46-52 (1)
46-52:⚠️ Potential issue | 🟡 Minor
ZoneId.systemDefault()사용은 운영 환경에서 예측 불가능한 동작을 유발할 수 있다.서버의 시스템 타임존에 의존하면, 멀티 리전 배포나 컨테이너 환경에서 인스턴스별로 타임존이 다를 경우 동일한 요청에 대해 서로 다른 쿼리 범위가 적용된다.
문제점(운영 관점):
- 장애 상황에서 시간 기반 쿼리 결과 불일치로 인해 디버깅이 어려워진다.
- 주문 조회 결과가 인스턴스에 따라 달라지면 사용자 불만 및 데이터 정합성 이슈가 발생한다.
수정안: 명시적인 타임존(UTC 또는 애플리케이션 설정 값)을 사용하거나, 클라이언트로부터 타임존을 전달받도록 변경한다.
- public ZonedDateTime startDateTime() { - return startDate != null ? startDate.atStartOfDay(ZoneId.systemDefault()) : null; - } - - public ZonedDateTime endDateTime() { - return endDate != null ? endDate.plusDays(1).atStartOfDay(ZoneId.systemDefault()) : null; - } + private static final ZoneId DEFAULT_ZONE = ZoneId.of("Asia/Seoul"); // 또는 UTC + + public ZonedDateTime startDateTime() { + return startDate != null ? startDate.atStartOfDay(DEFAULT_ZONE) : null; + } + + public ZonedDateTime endDateTime() { + return endDate != null ? endDate.plusDays(1).atStartOfDay(DEFAULT_ZONE) : null; + }추가 테스트: 타임존이 다른 환경에서 동일 날짜 입력에 대해 동일한 쿼리 범위가 생성되는지 단위 테스트로 검증해야 한다.
🤖 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/interfaces/api/experiment/OrderExperimentRequest.java` around lines 46 - 52, The startDateTime() and endDateTime() methods currently use ZoneId.systemDefault(), which leads to inconsistent query ranges across hosts; change them to use an explicit timezone (e.g., UTC) or the application-configured ZoneId instead of systemDefault — update startDateTime() and endDateTime() to accept or reference a fixed ZoneId (or a central config value) so date.atStartOfDay(...) and plusDays(1).atStartOfDay(...) use that explicit ZoneId; add unit tests that construct ZonedDateTime for differing system timezones to verify identical results for the same input date.docs/performance/product/explain-analysis.sql-88-91 (1)
88-91:⚠️ Potential issue | 🟡 Minor단건 상세 조회 시 soft-delete 조건 누락으로 분석 기준이 서비스 정책과 어긋날 수 있다.
운영 관점에서 Line 88-91은
id만 조건으로 사용하여 삭제된 상품까지 조회 대상으로 포함될 수 있다. 분석 문서의 기준 쿼리가 서비스 조회 정책(deleted_at IS NULL)과 다르면 캐시/인덱스 검증 결과가 실사용과 불일치할 수 있다.
수정안으로 단건 조회에도deleted_at IS NULL을 포함해 기준을 통일해야 한다.
추가 테스트로 삭제된 상품 ID와 미삭제 상품 ID 각각에 대해EXPLAIN및 결과 건수를 검증해야 한다.수정 예시
EXPLAIN SELECT * FROM products -WHERE id = 1; +WHERE id = 1 + AND deleted_at IS NULL;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/product/explain-analysis.sql` around lines 88 - 91, 단건 조회 기준 쿼리(EXPLAIN SELECT ... FROM products WHERE id = 1)에서 soft-delete 조건이 빠져 서비스 정책과 분석 기준이 불일치하므로 WHERE 절에 deleted_at IS NULL을 추가하여 삭제된 상품을 제외하도록 수정하고, 수정 후에는 삭제된 상품 ID와 미삭제 상품 ID 각각에 대해 EXPLAIN을 실행해 쿼리 플랜과 반환 건수(0 또는 1)를 검증해 문서의 분석 결과가 실사용과 일치하는지 확인하세요.docs/performance/order/seed-orders.sql-96-96 (1)
96-96:⚠️ Potential issue | 🟡 Minorproduct_count(5,000)가 실제 상품 수(100,000)와 불일치한다.
seed-products.sql에서 100,000개 상품을 생성하지만, 여기서는 5,000개만 참조한다. 이로 인해:
- 상품 5,001~100,000번은 order_item에서 참조되지 않음
- 인덱스 성능 테스트 시 데이터 분포가 왜곡될 수 있음
- product_id 기반 쿼리의 cardinality 테스트가 제한됨
🔧 상품 수와 일치하도록 수정
- DECLARE product_count INT DEFAULT 5000; + DECLARE product_count INT DEFAULT 100000;다만, 의도적으로 특정 상품에 주문을 집중시키려는 것이라면 주석으로 명시해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/order/seed-orders.sql` at line 96, The variable product_count in seed-orders.sql is set to 5,000 but seed-products.sql generates 100,000 products, causing a mismatch; update the DECLARE product_count INT DEFAULT 5000; to DECLARE product_count INT DEFAULT 100000; so order generation references the full product set (or, if the smaller value was intentional, add a clear comment above the DECLARE explaining why orders should be concentrated on the first 5,000 products), and verify any loops/functions that use product_count (e.g., order/item generation logic) still behave correctly with the larger value.docs/performance/order/seed-orders.sql-106-114 (1)
106-114:⚠️ Potential issue | 🟡 Minorproduct_id와 product_name이 서로 다른 RAND() 값을 사용하여 데이터 불일치가 발생한다.
FLOOR(1 + RAND() * product_count)가 두 번 호출되어 product_id와 product_name이 다른 상품을 가리킨다. 성능 테스트 시 JOIN 결과나 디버깅 시 혼란을 초래할 수 있다.🐛 RAND() 값을 변수에 저장하여 재사용
INSERT INTO order_item (order_id, product_id, product_name, price, quantity, created_at) SELECT o.id, - FLOOR(1 + RAND() * product_count), - CONCAT('Product_', LPAD(FLOOR(1 + RAND() * product_count), 6, '0')), + `@pid` := FLOOR(1 + RAND() * product_count), + CONCAT('Product_', LPAD(`@pid`, 6, '0')), ROUND(1000 + RAND() * 99000, 2), FLOOR(1 + RAND() * 5), o.created_at FROM orders o WHERE o.id BETWEEN v_current AND v_batch_end;동일한 수정이 아이템 2번(lines 117-126), 아이템 3번(lines 128-138)에도 필요하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/order/seed-orders.sql` around lines 106 - 114, The SELECT calls FLOOR(1 + RAND() * product_count) twice so product_id and product_name can point to different random products; compute the random index once and reuse it for both columns (e.g., assign the result of FLOOR(1 + RAND() * product_count) to a local expression/variable and reference that value for both product_id and product_name in the SELECT), and apply the same single-evaluation fix to the other two similar blocks later in the file (the other occurrences using FLOOR(1 + RAND() * product_count)).k6/list-test.js-16-21 (1)
16-21:⚠️ Potential issue | 🟡 MinorCursor 기준값(10000) 고정은 데이터 변화에 취약하다.
운영 관점에서 실제 최대 ID가 크게 다르면 빈 페이지 비율이 증가하거나 트래픽 분포가 왜곡된다. 수정안으로
CURSOR_MAX_ID를 환경변수로 분리하고 실행 환경별로 주입하는 방식이 안전하다.추가 테스트로
CURSOR_MAX_ID를 서로 다른 값(예: 10k/100k)으로 실행해 응답 건수와 p95 추이가 의도한 분포와 일치하는지 비교해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/list-test.js` around lines 16 - 21, Replace the hard-coded base value 10000 in getCursor() with a configurable integer (e.g., CURSOR_MAX_ID) read from an environment variable: add logic to parse process.env.CURSOR_MAX_ID (with a safe integer fallback if missing/invalid) and use that value instead of 10000 in the three return expressions inside getCursor(); ensure the parsing handles non-numeric input and negative values by falling back to a sensible default, and then run the k6 tests with different CURSOR_MAX_ID values (e.g., 10000 and 100000) to verify the resulting response counts and p95 match the intended distribution.apps/commerce-api/src/main/java/com/loopers/support/transaction/TransactionHelper.java-8-15 (1)
8-15:⚠️ Potential issue | 🟡 Minor트랜잭션 컨텍스트 검증 로직 누락
TransactionSynchronizationManager.registerSynchronization()은 활성 트랜잭션이 없을 때IllegalStateException을 발생시킨다. 호출부에서 실수로 트랜잭션 외부에서 호출할 경우, 런타임 예외가 발생하여 장애로 이어질 수 있다.🛡️ 방어 로직 추가 제안
public static void afterCommit(Runnable action) { + if (!TransactionSynchronizationManager.isSynchronizationActive()) { + throw new IllegalStateException("afterCommit은 활성 트랜잭션 내에서만 호출 가능하다"); + } TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { `@Override` public void afterCommit() { action.run(); } }); }추가 테스트: 트랜잭션 외부에서
afterCommit호출 시 명확한 예외 메시지가 출력되는지 단위 테스트로 검증해야 한다.🤖 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/support/transaction/TransactionHelper.java` around lines 8 - 15, The afterCommit method must guard against being called outside a transaction: before invoking TransactionSynchronizationManager.registerSynchronization(...) check TransactionSynchronizationManager.isSynchronizationActive() and if false throw a clear IllegalStateException (e.g., "afterCommit must be called within an active transaction") instead of letting registerSynchronization raise a generic exception; update TransactionHelper.afterCommit to perform this check and keep the existing TransactionSynchronization implementation, and add a unit test that calls TransactionHelper.afterCommit outside of a transaction and asserts the specific IllegalStateException message.modules/redis/src/main/java/com/loopers/config/redis/RedisConfig.java-45-52 (1)
45-52:⚠️ Potential issue | 🟡 MinorpubSubRedisConnectionFactory가 다른 연결 팩토리와 일관성 없게 구현되었다.
이 팩토리는
lettuceConnectionFactory()헬퍼 메서드를 사용하지 않고 직접RedisStandaloneConfiguration을 생성하여,defaultRedisConnectionFactory()와masterRedisConnectionFactory()와 다른 패턴으로 작동한다. 일관된 구성 및 향후 타임아웃/재시도 설정 추가를 고려할 때 동일한 헬퍼 메서드를 사용하도록 리팩토링해야 한다.🔧 헬퍼 메서드 사용으로 일관성 개선 제안
`@Qualifier`(CONNECTION_PUB_SUB) `@Bean` public LettuceConnectionFactory pubSubRedisConnectionFactory() { RedisNodeInfo master = redisProperties.master(); - RedisStandaloneConfiguration standaloneConfig = new RedisStandaloneConfiguration(master.host(), master.port()); - standaloneConfig.setDatabase(redisProperties.database()); - return new LettuceConnectionFactory(standaloneConfig); + int database = redisProperties.database(); + List<RedisNodeInfo> replicas = redisProperties.replicas(); + return lettuceConnectionFactory( + database, master, replicas, + null // Pub/Sub는 특별한 readFrom 설정 불필요 + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/redis/src/main/java/com/loopers/config/redis/RedisConfig.java` around lines 45 - 52, pubSubRedisConnectionFactory currently builds a RedisStandaloneConfiguration directly, diverging from the pattern used by defaultRedisConnectionFactory() and masterRedisConnectionFactory(); refactor pubSubRedisConnectionFactory() to call the existing lettuceConnectionFactory(...) helper (passing the master RedisNodeInfo.host(), master.port(), and redisProperties.database() or appropriate RedisNodeInfo) so all three factories use the same helper and retain the CONNECTION_PUB_SUB qualifier; this ensures consistent settings and centralizes future timeout/retry config changes.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java-88-97 (1)
88-97:⚠️ Potential issue | 🟡 Minor캐시 키 생성 시 Sort.toString() 사용을 피하고 명시적 직렬화를 사용해야 한다.
pageable.getSort().toString()결과는 주어진 Sort 객체에 대해 일관성이 있지만, Spring Data는 이 메서드를 직렬화 목적으로 사용하지 않기를 권장한다. 다양한 방식으로 구성된 동일한 정렬 조건(예:Sort.by("field").descending()과Sort.by(DESC, "field"))이 서로 다른 문자열을 생성할 수 있어 캐시 미스가 발생할 위험이 있다.현재 테스트는 하드코딩된 문자열로만 검증하며, 실제 Pageable 객체의 다양한 구성 방식에 따른 캐시 동작을 검증하지 않는다. Sort를 정규화된 형식으로 명시적으로 직렬화하거나, Sort 객체의 주요 필드(direction, properties)를 개별적으로 캐시 키에 포함하도록 변경하기를 권장한다.
🤖 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/product/ProductFacade.java` around lines 88 - 97, The cache key currently uses pageable.getSort().toString(), which is unstable; change getActiveList to build a normalized sort representation (e.g., iterate pageable.getSort().stream() or pageable.getSort().toList() and join each Order's property and direction in a canonical order) and pass that normalizedSort to productCacheManager.getList instead of Sort.toString(); update the call sites and tests that assert caching to use different Sort constructions (Sort.by("a").descending() vs Sort.by(DESC, "a")) to verify normalization; reference getActiveList, productCacheManager.getList, and CachedPage when making these changes.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.java-57-64 (1)
57-64:⚠️ Potential issue | 🟡 Minor
evictAllLists()에서 Race Condition 가능성이 있다.
members()호출과delete()호출 사이에 새로운 캐시 엔트리가 추가될 수 있다. 새로 추가된 엔트리는 삭제되지 않지만 레지스트리는 삭제되어, 다음evictAllLists()호출 시 해당 키가 삭제되지 않는다. 현재 TTL이 5분이고 admin 작업 빈도가 낮아 실제 영향은 제한적이나, 인지하고 있어야 한다.🤖 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/product/ProductCacheManager.java` around lines 57 - 64, evictAllLists() has a race between redisTemplate.opsForSet().members(LIST_KEYS_REGISTRY) and redisTemplate.delete(...) where new keys can be added and escape deletion; replace the two-step approach with an atomic Redis operation (e.g. execute a Lua script via redisTemplate.execute or a RedisCallback) that reads SMEMBERS LIST_KEYS_REGISTRY, deletes all returned keys and then deletes LIST_KEYS_REGISTRY in one atomic script, then call publishInvalidation("list:all") after the script completes; update ProductCacheManager.evictAllLists to use that Lua script/RedisCallback instead of separate members() and delete() calls.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheInvalidationConfig.java-53-55 (1)
53-55:⚠️ Potential issue | 🟡 Minor
Long.parseLong예외 처리 누락으로 장애 전파 위험이 있다.
message.substring("detail:".length())가 숫자가 아닌 경우NumberFormatException이 발생하여 메시지 처리가 실패한다. Redis Pub/Sub 메시지가 손상되거나 잘못된 형식일 경우, 후속 메시지 처리도 영향받을 수 있다.수정 제안: 방어적 파싱 추가
if (message.startsWith("detail:")) { String productIdStr = message.substring("detail:".length()); - localCacheManager.evictDetail(Long.parseLong(productIdStr)); + try { + localCacheManager.evictDetail(Long.parseLong(productIdStr)); + } catch (NumberFormatException e) { + log.warn("캐시 무효화 메시지 파싱 실패: {}", message, e); + } } else if ("list:all".equals(message)) {🤖 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/product/ProductCacheInvalidationConfig.java` around lines 53 - 55, The substring extraction in ProductCacheInvalidationConfig that calls Long.parseLong on message.substring("detail:".length()) can throw NumberFormatException and crash message processing; wrap the parse in defensive logic (e.g., trim the substring and validate it is a numeric value or use a try/catch around Long.parseLong) and on parse failure log a warning including the offending message and skip calling localCacheManager.evictDetail so processing continues safely without throwing.apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.java-109-109 (1)
109-109:⚠️ Potential issue | 🟡 Minor오프셋 캐시 히트 시 Page 정렬 메타데이터가 유실된다
운영 관점에서 캐시 미스(DB 경로)와 캐시 히트 경로의 응답 메타데이터가 달라지면, 클라이언트의 페이지네이션 디버깅 난이도가 크게 올라간다.
Line 109/130/137의PageRequest.of(cp.page(), cp.size())를PageRequest.of(cp.page(), cp.size(), pageable.getSort())로 바꿔 정렬 정보를 보존하는 것이 필요하다.
추가 테스트로 동일 요청에 대해 캐시 미스 1회 후 캐시 히트 시Page.getSort()가 동일한지 검증해야 한다.Also applies to: 130-130, 137-137
🤖 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/product/ProductExperimentFacade.java` at line 109, The cached-offset path returns a PageImpl without preserving sort metadata causing different responses between DB (miss) and cache (hit); update the PageImpl construction in ProductExperimentFacade where you build new PageImpl<>(cp.content(), PageRequest.of(cp.page(), cp.size()), cp.totalElements()) to include the original sort: use PageRequest.of(cp.page(), cp.size(), pageable.getSort()) (apply the same change at the other occurrences on the same method/lines), and add a test that performs one cache-miss then a cache-hit for the same request and asserts Page.getSort() is equal between responses.
🧹 Nitpick comments (22)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentRequest.java (1)
42-44:toPageable()메서드가 세 개의 레코드에 중복 구현되어 있다.현재는 정렬 기준이 동일하나, 향후 정렬 기준 변경 시 세 곳을 모두 수정해야 하며 누락 가능성이 있다.
수정안(선택적): 공통 유틸리티 메서드나 상위 타입으로 추출을 고려할 수 있다. 단, 실험 코드이고 정렬 기준이 달라질 가능성이 있다면 현행 유지도 무방하다.
Also applies to: 71-73, 92-94
🤖 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/interfaces/api/experiment/OrderExperimentRequest.java` around lines 42 - 44, Multiple classes implement an identical toPageable() that returns PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "createdAt")); extract that logic into a single reusable location (either a static utility method like PaginationUtils.toPageable(page, size, sortField) or a protected method in a shared base class used by OrderExperimentRequest and the other experiment request classes) and replace the duplicated toPageable() implementations with calls to that shared method; ensure the new helper accepts or defaults the sort field ("createdAt") so future changes require editing only one place.docs/performance/product/explain-analysis.sql (1)
17-17: OFFSET 0만으로는 후반 페이지 성능 리스크를 검증할 수 없다.운영 관점에서 모든 목록 시나리오가
OFFSET 0만 사용하면 실제 서비스의 deep pagination 구간에서 발생하는 응답 지연과 스캔 증가를 놓치게 된다.
수정안으로 고오프셋 시나리오와 keyset pagination 비교 시나리오를 문서에 추가하는 것이 바람직하다.
추가 테스트로OFFSET 10000/50000대비 keyset 쿼리의rows examined와 실행시간을 함께 기록해야 한다.추가 시나리오 예시
+-- 9. Deep pagination 검증 (OFFSET 큰 값) +EXPLAIN ANALYZE +SELECT * +FROM products +WHERE deleted_at IS NULL +ORDER BY created_at DESC +LIMIT 20 OFFSET 10000; + +-- 10. Keyset pagination 비교 +EXPLAIN ANALYZE +SELECT * +FROM products +WHERE deleted_at IS NULL + AND created_at < '2026-03-01 00:00:00' +ORDER BY created_at DESC +LIMIT 20;Also applies to: 29-29, 39-39, 51-51, 61-61, 73-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/product/explain-analysis.sql` at line 17, 현재 문서의 쿼리들이 LIMIT 20 OFFSET 0; (및 동일한 패턴이 사용된 다른 쿼리들)만 테스트하고 있어 deep pagination 성능 리스크를 검증하지 못합니다; 수정 방법은 각 해당 쿼리(식별자: LIMIT 20 OFFSET 0)를 복제해 고오프셋 케이스(예: OFFSET 10000, OFFSET 50000)를 추가하고, 동일한 결과를 반환하는 keyset pagination 버전(예: WHERE id > :last_id LIMIT 20 형태)을 추가하여 비교 테스트를 수행한 뒤 각 케이스에 대해 rows examined와 실행시간을 함께 기록하도록 문서의 테스트 항목과 결과 표를 확장하세요.docs/performance/product/seed-products.sql (1)
47-53: MySQL 8.0에서 사용자 정의 변수(@rownum) 패턴은 deprecated 되었다.
@rownum :=@rownum+ 1패턴은 MySQL 8.0에서 평가 순서가 정의되지 않아 예기치 않은 결과를 발생시킬 수 있다. 현재 로컬 테스트 환경에서는 동작하지만, MySQL 버전 업그레이드 또는 옵티마이저 변경 시 문제가 될 수 있다.MySQL 8.0+에서는
ROW_NUMBER()윈도우 함수 또는 재귀 CTE를 권장한다.♻️ 재귀 CTE를 사용한 대안
FROM ( - SELECT `@rownum` := `@rownum` + 1 AS seq - FROM information_schema.columns a - CROSS JOIN information_schema.columns b - CROSS JOIN (SELECT `@rownum` := i - 1) r - LIMIT 1000 + WITH RECURSIVE seq_cte AS ( + SELECT i AS seq + UNION ALL + SELECT seq + 1 FROM seq_cte WHERE seq < i + 999 + ) + SELECT seq FROM seq_cte ) t;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/product/seed-products.sql` around lines 47 - 53, Replace the deprecated user-variable row counter (`@rownum` := `@rownum` + 1) in the subquery that produces seq (alias t) with a deterministic approach: use the ROW_NUMBER() window function (ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) AS seq) or a recursive CTE that generates 1000 rows and yields seq, and remove any reference to `@rownum`; update the SELECT ... FROM (...) t block accordingly so seq is produced reliably without user variables.docs/performance/order/seed-orders.sql (1)
63-69: @rownum 패턴은 MySQL 8.0에서 deprecated 되었다.
seed-products.sql과 동일하게@rownum :=@rownum+ 1패턴이 사용되었다. MySQL 8.0+에서는 평가 순서가 보장되지 않으므로 재귀 CTE 또는 ROW_NUMBER() 윈도우 함수로 대체를 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/order/seed-orders.sql` around lines 63 - 69, The query uses the deprecated user-variable pattern "@rownum := `@rownum` + 1" to produce seq; replace it with a deterministic ROW_NUMBER() or a recursive CTE. Locate the subquery that defines "seq" (the block using "@rownum" and alias "t") and rewrite it to use either ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) applied to the derived cross-join set, or replace the whole subquery with a WITH RECURSIVE generator that produces 1..1000 and aliases the column as seq; ensure all references to "@rownum" are removed and the column name "seq" is preserved.k6/detail-test.js (1)
31-38: 워밍업이 랜덤 기반이라 실험 분산이 커질 수 있다.운영 관점에서 워밍업 커버리지가 실행마다 달라지면 동일 조건 비교가 어려워진다. 수정안으로 워밍업은 상위 HOT 키를 순차적으로 고정 순회(
__ITER % HOT_PRODUCT_IDS.length)하도록 바꾸는 것이 좋다.추가 테스트로 동일 환경에서 3회 반복 실행 시
v2_detail_duration/v3_detail_duration의 p50 편차가 허용 범위 내인지 검증해야 한다.Also applies to: 47-54, 83-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/detail-test.js` around lines 31 - 38, The warmup stage is currently selecting keys randomly which increases variance; update the warmup functions (warmupV2 and warmupV3) used by the v2_warmup/v3_warmup executors so they deterministically pick HOT keys by computing an index from the iteration counter (use __ITER % HOT_PRODUCT_IDS.length) and iterate them in fixed order instead of random selection; apply the same change to the other warmup blocks that call warmupV2/warmupV3; after fixing, run the test suite 3 times under identical conditions and compare v2_detail_duration and v3_detail_duration p50 to ensure variance is within acceptable bounds.k6/list-test.js (1)
33-119: 임계치(thresholds) 부재로 성능 회귀 자동 감지가 어렵다.운영 관점에서 임계치가 없으면 실험 결과를 수동 해석해야 하며 회귀가 CI에서 자동 차단되지 않는다. 수정안으로 주요 Trend와 실패율(
http_req_failed) 기준을thresholds에 추가해야 한다.수정 예시
export const options = { summaryTrendStats: ['avg', 'min', 'med', 'max', 'p(50)', 'p(95)', 'p(99)'], + thresholds: { + http_req_failed: ['rate<0.01'], + v1_offset_duration: ['p(95)<300'], + v2_offset_duration: ['p(95)<150'], + v3_offset_duration: ['p(95)<80'], + v1_cursor_duration: ['p(95)<300'], + v2_cursor_duration: ['p(95)<150'], + v3_cursor_duration: ['p(95)<80'], + }, scenarios: {추가 테스트로 의도적으로 느린 환경(예: 제한된 DB 자원)에서 실행해 thresholds 위반 시 즉시 실패 처리되는지 검증해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/list-test.js` around lines 33 - 119, Add k6 thresholds to the exported options so CI can auto-detect regressions: update the top-level options object (next to summaryTrendStats and scenarios) to include a thresholds map that defines SLA limits for key metrics such as 'http_req_failed' (e.g. rate < 1%), 'http_req_duration' (e.g. p(95) < 500, avg < 200), and any important iteration/transaction metrics you use; reference the existing summaryTrendStats and scenario names (v1_offset, v2_cursor, testOffsetV2, etc.) when choosing which metrics to protect. Ensure thresholds use k6 metric keys (http_req_failed, http_req_duration, iteration_duration or a custom check/count) and run the suite against a deliberately slowed environment to confirm threshold violations cause the run to fail.apps/commerce-api/src/test/java/com/loopers/application/order/OrderServiceIntegrationTest.java (1)
123-127: OrderStatus 필터링 테스트 케이스 누락현재 테스트는
status = null인 경우만 검증하고 있다. 새로 추가된 상태 기반 필터링 기능에 대한 테스트가 없어, 특정 상태로 필터링 시 발생할 수 있는 버그를 사전에 탐지하지 못한다.🧪 상태 필터링 테스트 추가 제안
`@Test` void 특정_상태로_필터링하면_해당_상태의_주문만_반환한다() { // PENDING 상태 주문 생성 orderService.createOrder(OrderCommand.Create.of(1L, List.of( OrderCommand.CreateItem.of(1L, "운동화", new BigDecimal("50000"), 1) ))); // 필요 시 일부 주문 상태 변경 (COMPLETED, CANCELLED 등) Page<Order> result = orderService.findOrdersByUserIdAndStatusAndDateRange( 1L, null, null, OrderStatus.PENDING, PageRequest.of(0, 20)); assertThat(result.getContent()).allMatch(order -> order.getStatus() == OrderStatus.PENDING); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/order/OrderServiceIntegrationTest.java` around lines 123 - 127, The test suite is missing a case that verifies status-based filtering for findOrdersByUserIdAndStatusAndDateRange; add a new test in OrderServiceIntegrationTest that creates orders with known statuses (e.g., create a PENDING order via orderService.createOrder and ensure some others are COMPLETED/CANCELLED or update their status), call orderService.findOrdersByUserIdAndStatusAndDateRange(1L, null, null, OrderStatus.PENDING, PageRequest.of(0,20)), and assert that every returned Order has getStatus() == OrderStatus.PENDING (and optionally assert expected count). This ensures the status parameter is honored by the method.apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.java (1)
22-24: 복합 인덱스 설계 적절함
(product_id, order_id)순서의 복합 인덱스는 상품별 주문 조회 쿼리에 적합하다. 운영 시 고려사항:
- 인덱스 추가 시점: 대용량 테이블에서 인덱스 생성은 락 경합을 유발할 수 있으므로, 운영 배포 시 별도 마이그레이션 스크립트로
ALGORITHM=INPLACE, LOCK=NONE옵션 사용을 권장한다.- 기존 인덱스 중복 검토:
order_id단독 인덱스가 이미 존재하는 경우 커버링 인덱스 효과를 위해 통합 검토가 필요하다.🤖 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/order/OrderItem.java` around lines 22 - 24, The composite index declared on the OrderItem entity (`@Table` indexes = `@Index`(name = "idx_order_item_product", columnList = "product_id, order_id")) is fine for product-based queries but needs operational handling: create a dedicated DB migration (not code change in OrderItem) that adds the composite index using ALGORITHM=INPLACE, LOCK=NONE to avoid locks during production, and inspect existing single-column indexes (e.g., any existing index on order_id) to decide whether to drop or consolidate them into the composite index to avoid redundancy and preserve covering index benefits.apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderRequest.java (1)
108-122: Bean Validation 사용에 대한 프로젝트 가이드라인 확인 필요기존 학습 내용에 따르면, 이 프로젝트에서는 DTO에 Bean Validation 어노테이션을 사용하지 않고 도메인 레이어에서 검증하도록 권장하고 있다. 기존 코드와 일관성을 유지하고 있으나, 프로젝트 방향성에 따라 도메인 레이어로 검증 로직 이동을 검토할 수 있다.
운영 관점에서 Bean Validation 사용 시
MethodArgumentNotValidException이 발생하며, 이를CoreException으로 통합 처리하지 않으면 일관되지 않은 에러 응답 형식이 반환될 수 있다.Based on learnings: "Do not use Bean Validation annotations on DTOs in this project. Move validation logic into the domain layer."
🤖 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/interfaces/api/order/OrderRequest.java` around lines 108 - 122, The ListByProduct record currently uses Bean Validation annotations on its fields (productId, page, size); remove the `@NotNull/`@PositiveOrZero/@Min/@Max annotations from the ListByProduct record and implement equivalent validation in the domain layer (e.g., where ListByProduct is converted/used) so productId, page (default 0), and size (default 20) are validated there; ensure the domain validation throws the project's CoreException on invalid input so error responses remain consistent, and keep the toPageable() method intact to use the validated page and size values.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (1)
74-84: Cache stampede 미대응은 알려진 제약사항이다.캐시 미스 시 다수의 요청이 동시에 DB를 조회할 수 있는 stampede 현상에 대한 방어가 없다. PR 목표에 명시된 대로 분산 락 미적용 상태이며, 트래픽 증가 시 DB 부하 급증 가능성이 있다.
운영 환경 투입 전 다음 사항을 고려해야 한다:
- 캐시 미스율 메트릭 모니터링 추가
- 임계치 초과 시 분산 락 또는 request coalescing 도입 검토
🤖 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/product/ProductFacade.java` around lines 74 - 84, The getActiveDetail method currently returns from productCacheManager.getDetail on miss by directly calling productService.getActiveProduct, which allows concurrent requests to trigger a DB hit (cache stampede); update getActiveDetail to implement protection on cache misses (e.g., request coalescing or distributed locking) around productCacheManager.putDetail so only one request fetches and populates the cache while others wait or receive the populated value, and add a metric emission for cache miss rate inside productCacheManager.getDetail/getActiveDetail to alert when miss rate exceeds a threshold; focus changes around getActiveDetail, productCacheManager (getDetail/putDetail) and productService.getActiveProduct.apps/commerce-api/src/main/java/com/loopers/domain/order/OrderStatus.java (1)
3-9: 상태 전이 규칙 검증 로직 추가를 고려할 수 있다.현재 enum은 단순 값 나열만 포함한다. 운영 환경에서 잘못된 상태 전이(예:
CANCELLED→DELIVERED)가 발생할 경우 데이터 정합성 문제가 생길 수 있다.상태 전이 규칙을 enum 내부 또는 별도 도메인 서비스에서 검증하는 방식을 고려할 수 있다.
♻️ 상태 전이 검증 메서드 예시
public enum OrderStatus { PENDING, PAID, SHIPPING, DELIVERED, CANCELLED; public boolean canTransitionTo(OrderStatus next) { return switch (this) { case PENDING -> next == PAID || next == CANCELLED; case PAID -> next == SHIPPING || next == CANCELLED; case SHIPPING -> next == DELIVERED || next == CANCELLED; case DELIVERED, CANCELLED -> false; }; } }🤖 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/order/OrderStatus.java` around lines 3 - 9, OrderStatus currently only lists values which allows invalid transitions; add a transition-validation method (e.g., public boolean canTransitionTo(OrderStatus next)) on the OrderStatus enum (or implement an equivalent domain service) that encodes allowed transitions: PENDING -> PAID|CANCELLED, PAID -> SHIPPING|CANCELLED, SHIPPING -> DELIVERED|CANCELLED, and DELIVERED/CANCELLED -> none, and update any state-change code paths to call OrderStatus.canTransitionTo(next) and reject or throw when the transition is invalid.apps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductCacheManagerIntegrationTest.java (2)
36-80: 상세 캐시 테스트 - 역직렬화 실패 시나리오 테스트 추가를 권장한다.현재 테스트는 정상 케이스와 캐시 미스 케이스를 다루고 있으나, Redis에 잘못된 JSON이 저장된 경우
getDetail()이Optional.empty()를 반환하는 방어 로직에 대한 테스트가 없다. 운영 환경에서 캐시 데이터 손상 시 graceful degradation 동작을 검증해야 한다.♻️ 역직렬화 실패 테스트 추가 예시
`@Test` void 잘못된_JSON이_저장된_경우_빈_Optional을_반환한다() { // given Long productId = 1L; // Redis에 직접 잘못된 JSON 저장 redisTemplate.opsForValue().set("product:detail:" + productId, "invalid-json"); // when Optional<ProductInfo> cached = productCacheManager.getDetail(productId); // then assertThat(cached).isEmpty(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductCacheManagerIntegrationTest.java` around lines 36 - 80, Add a test that verifies ProductCacheManager.getDetail(...) returns Optional.empty() when Redis contains invalid JSON: insert a malformed value into the same Redis key used by ProductCacheManager (e.g., "product:detail:" + productId) via redisTemplate.opsForValue().set(..., "invalid-json"), then call productCacheManager.getDetail(productId) and assert the result is empty; place this test alongside the other detail-cache tests (same nested class) to cover the deserialization failure path.
150-159: 테스트 헬퍼 메서드 개선을 권장한다.
createProductInfo()가 고정된 timestamp를 사용하는 것은 테스트 목적상 문제없으나, 다른 필드(brandId, name 등)도 id에 따라 구분 가능하도록 하면 목록 캐시 테스트에서 데이터 구분이 명확해진다.♻️ 헬퍼 메서드 개선 예시
private ProductInfo createProductInfo(Long id) { return new ProductInfo( - id, 1L, "테스트 브랜드", "테스트 상품", + id, id, "테스트 브랜드 " + id, "테스트 상품 " + id, new BigDecimal("10000"), 100, "설명", 0, ProductInfo.Status.ACTIVE, LocalDateTime.of(2024, 1, 1, 0, 0), LocalDateTime.of(2024, 1, 1, 0, 0), null ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductCacheManagerIntegrationTest.java` around lines 150 - 159, The helper createProductInfo currently returns ProductInfo with many constant fields which makes different products hard to distinguish in list/cache tests; modify createProductInfo(Long id) so that fields vary by id (e.g., set brandId = id, brand/name = "테스트 브랜드-"+id and "테스트 상품-"+id, price or stock can incorporate id or id.hashCode(), and timestamps can be derived from id or use LocalDateTime.now().minusSeconds(id) if reproducibility isn't required) while still returning a valid ProductInfo instance; update references to ProductInfo constructors used in ProductCacheManagerIntegrationTest to use the new varying values so tests can clearly distinguish items.apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentV1Dto.java (1)
35-57: API 응답에OrderStatusenum 직접 노출 시 하위 호환성을 고려해야 한다.
OrderStatusenum이 API 응답에 직접 노출되어 있다. enum 값이 변경되거나 추가될 경우 API 클라이언트에 영향을 줄 수 있다. 실험용 API이므로 현재는 허용 가능하나, 정식 API로 전환 시 String 변환 또는 별도 API 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/interfaces/api/experiment/OrderExperimentV1Dto.java` around lines 35 - 57, AdminOrderResponse가 OrderStatus enum을 직접 노출하고 있어 하위호환성 문제가 발생할 수 있으니 필드 타입을 enum에서 String으로 변경하고 변환 로직을 추가하세요: 변경 대상은 record AdminOrderResponse의 선언과 정적 팩토리 메서드 from(OrderInfo.OrderAdminSummary)이며, 필드 타입을 OrderStatus에서 String으로 바꾸고 from에서는 summary.status().name() (또는 필요시 .toString()/커스텀 매핑)으로 변환해 전달하도록 수정하고, 이 변경에 따라 AdminOrderResponse를 소비하는 코드(직렬화/문서화 레이어 등)를 함께 업데이트하세요.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.java (1)
106-109: 캐시 키 포맷에 구분자 충돌 가능성이 있다.
sort파라미터에:가 포함될 수 있어(예: "createdAt: DESC") 키 파싱 시 모호함이 발생할 수 있다. 현재는 키를 파싱할 필요가 없으므로 문제없으나, 향후 유지보수를 위해 더 명확한 구분자 사용을 권장한다.♻️ 개선 예시
private String listKey(Long brandId, String sort, int page, int size) { String brandPart = brandId != null ? brandId.toString() : "all"; - return LIST_KEY_PREFIX + brandPart + ":" + sort + ":" + page + ":" + size; + // sort에서 공백과 콜론을 제거하거나, 다른 구분자 사용 + String normalizedSort = sort.replace(":", "").replace(" ", "_"); + return LIST_KEY_PREFIX + brandPart + "|" + normalizedSort + "|" + page + "|" + size; }🤖 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/product/ProductCacheManager.java` around lines 106 - 109, The current listKey method builds cache keys with ":" separators which can collide if the sort string contains ":" (e.g., "createdAt: DESC"); update listKey to produce unambiguous keys by encoding or escaping the sort value before concatenation (for example use URLEncoder.encode(sort, StandardCharsets.UTF_8) or Base64 URL-safe encoding) so the delimiter (LIST_KEY_PREFIX + brandPart + ":" + encodedSort + ":" + page + ":" + size) cannot be confused with characters inside sort; modify the listKey method to perform this encoding and add a brief comment mentioning the chosen encoding to aid future maintenance.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheInvalidationConfig.java (1)
56-61: 알 수 없는 메시지 형식에 대한 처리가 없다.
detail:,list:all,all외의 메시지는 무시된다. 의도적인 설계라면 로그로 기록하여 모니터링 가시성을 확보하는 것이 운영에 유리하다. 확장성을 고려하면 현재 구조는 수용 가능하나, 디버깅 시 누락된 메시지 추적이 어려울 수 있다.선택적 개선: 알 수 없는 메시지 로깅
} else if ("all".equals(message)) { localCacheManager.evictAllDetails(); localCacheManager.evictAllLists(); + } else { + log.debug("처리되지 않은 캐시 무효화 메시지: {}", message); }🤖 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/product/ProductCacheInvalidationConfig.java` around lines 56 - 61, ProductCacheInvalidationConfig currently handles "detail:", "list:all", and "all" messages but silently ignores any other message; update the message-handling block to add a final else branch that logs unrecognized messages (include the raw message and any relevant context like topic or key) using the class logger so operators can monitor and debug unexpected payloads—locate the conditional that checks message values and add an else that calls the logger (e.g., logger.warn or logger.info) with the message and context, leaving existing calls to localCacheManager.evictAllDetails()/evictAllLists() unchanged.apps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentController.java (2)
57-79:@Valid사용이 프로젝트 에러 처리 패턴과 충돌할 수 있다.프로젝트 컨벤션에 따르면 Bean Validation 대신 도메인 레이어에서 검증하고
CoreException을 통해ApiControllerAdvice로 일관된 에러 응답을 생성해야 한다.@Valid사용 시MethodArgumentNotValidException이 발생하여 에러 응답 형식이 불일치할 수 있다.실험용 컨트롤러라면 수용 가능하나, 프로덕션 전환 시 검증 로직을 파사드/서비스 레이어로 이동해야 한다.
Based on learnings: "Do not introduce MethodArgumentNotValidException handlers or Bean Validation handling, as that would create inconsistent error handling patterns."
🤖 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/interfaces/api/experiment/ProductExperimentController.java` around lines 57 - 79, Remove the `@Valid` annotation from the controller methods listOffsetV1, listOffsetV2, and listOffsetV3 and shift request validation into the domain/facade layer (e.g., inside ProductExperimentRequest.ListOffset validation logic or within experimentFacade.getListOffsetV1/getListOffsetV2/getListOffsetV1 methods) so violations throw CoreException (or the project’s domain validation exceptions) instead of MethodArgumentNotValidException; ensure the controller continues to call request.brandId() and request.toPageable() but rely on the facade to validate and throw CoreException for inconsistent input to preserve the ApiControllerAdvice error contract.
115-118:/cache-stats엔드포인트에 접근 제어가 없다.캐시 통계 정보(hit rate, eviction count 등)가 인증 없이 노출된다. 실험 환경에서는 수용 가능하나, 프로덕션 배포 시 관리자 권한 검증 또는 actuator 엔드포인트로 이동이 필요하다.
🤖 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/interfaces/api/experiment/ProductExperimentController.java` around lines 115 - 118, The /cache-stats endpoint in ProductExperimentController (method cacheStats calling experimentFacade.getCacheStats) is unprotected and exposes cache metrics; restrict access by adding an authorization check (e.g., require ADMIN role via `@PreAuthorize` or `@Secured` on cacheStats) or move this functionality to a protected actuator endpoint; update ProductExperimentController.cacheStats to validate the current user's authority before returning experimentFacade.getCacheStats (or relocate the logic to an actuator-managed bean) so only authorized admins can access the data.docs/performance/order/explain-analysis.sql (1)
86-99: 데이터 분포에 따른 성능 양극화 분석이 운영에 유용하다.SHIPPING(9%)과 DELIVERED(54%) 간 rows 추정치 차이(90,062 vs 249,006)를 기록하고, LIMIT + 인덱스 정렬 순서로 Early Termination이 보장됨을 명시했다. 운영 중 특정 상태의 비율이 변동할 경우 쿼리 성능이 달라질 수 있으므로, 상태별 분포 모니터링 지표 추가를 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance/order/explain-analysis.sql` around lines 86 - 99, Add a short operational recommendation under the Scenario 7/8 analysis: state the need to monitor per-status data distribution and its effect on index selectivity, referencing the queries/indices shown (the SELECT ... WHERE status = 'SHIPPING' / 'DELIVERED' examples, idx_orders_status_created, and the LIMIT 20 / Early Termination behavior). Specify what to monitor (e.g., percent of orders per status and time-series trend) and suggest alert thresholds or a rolling-window metric so operators can detect distribution shifts that would impact the observed rows estimates and query performance.apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderJpaRepository.java (1)
43-47:findAllByProductId쿼리의 DISTINCT + JOIN 페이징 특성을 인지해야 한다.
DISTINCT o+JOIN o.orderItems+ORDER BY o.createdAt DESC+ 페이징 조합은 MySQL이 전체 결과를 정렬 후 DISTINCT 처리해야 할 수 있다. explain-analysis.sql 시나리오 9에서 filesort가 발생하나, covering index로 order_item 테이블 접근이 없고 결과 건수가 적어 수용 가능하다고 분석되어 있다.대량 데이터 환경에서 특정 상품의 주문 건수가 급증할 경우 성능 저하 가능성이 있으므로 모니터링이 필요하다.
🤖 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/order/OrderJpaRepository.java` around lines 43 - 47, The JPQL using "SELECT DISTINCT o FROM Order o JOIN o.orderItems oi ... ORDER BY o.createdAt DESC" in findAllByProductId can cause MySQL filesort/inefficient DISTINCT+JOIN paging at scale; change the query to avoid JOIN+DISTINCT by using an EXISTS subquery (e.g., "SELECT o FROM Order o WHERE EXISTS (SELECT 1 FROM OrderItem oi WHERE oi.order = o AND oi.productId = :productId) ORDER BY o.createdAt DESC") or use GROUP BY o.id to deduplicate, and add a supporting DB index on order_items(product_id, order_id) (or product_id, order_id, created_at) to improve performance and make paging deterministic; update the repository method findAllByProductId accordingly and monitor query plans under heavy load.apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.java (1)
164-174: 커서 캐시에서totalElements를hasNext용도로 재사용하는 구조는 분리하는 편이 안전하다운영 관점에서 의미가 다른 필드를 혼용하면 이후 기능 확장 시 잘못된 페이지 계산/모니터링 지표 왜곡으로 이어지기 쉽다.
수정안으로 커서 전용 캐시 DTO(content,hasNext,nextCursor)를 별도로 두고,CachedPage.totalElements재사용을 제거하는 것을 권장한다.
추가 테스트로 Redis/L1 캐시 직렬화-역직렬화 후에도 캐시 히트 결과가 DB 미스 결과와 완전히 동일한지 확인하는 회귀 테스트를 추가하면 좋다.Also applies to: 198-208
🤖 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/product/ProductExperimentFacade.java` around lines 164 - 174, The code is reusing CachedPage.totalElements as a hasNext flag which mixes semantics; update ProductExperimentFacade to stop repurposing CachedPage.totalElements by introducing a cursor-specific cache DTO (e.g., CursorCachedPage or CursorCacheDTO containing content, hasNext, nextCursor) and change the flow around toCursorResult, productCacheManager.putList, and CachedPage usage to store the new DTO for cursor pagination; modify toCursorResult (and any caller like productService.findActiveProductsCursor) to return/consume the explicit hasNext field and ensure productCacheManager.putList writes the cursor DTO instead of setting totalElements as 1/0; add/adjust (de)serialization and unit/regression tests to confirm cache hits after Redis/L1 serialize-deserialize behave identically to DB results.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java (1)
36-38: 벌크 업데이트 뒤 1차 캐시를 비우지 않으면 같은 트랜잭션에서 stale entity를 읽는다.
softDeleteByIds()는 JPQL 벌크 업데이트라 영속성 컨텍스트를 우회한다. 같은 트랜잭션에서 이미 로드된Product를 재사용하면 삭제 직후 후속 로직이나 이벤트 페이로드가 삭제 전 상태를 기준으로 만들어질 수 있다.@Modifying(clearAutomatically = true, flushAutomatically = true)를 적용하거나, 호출 직후EntityManager.clear()를 보장하도록 수정해 달라. 같은 트랜잭션에서findById()후softDeleteByIds()를 호출했을 때deletedAt이 즉시 반영되는 테스트를 추가해 달라.수정 예시
- `@Modifying` + `@Modifying`(flushAutomatically = true, clearAutomatically = true) `@Query`("UPDATE Product p SET p.deletedAt = CURRENT_TIMESTAMP WHERE p.id IN :ids") int softDeleteByIds(`@Param`("ids") List<Long> ids);As per coding guidelines,
**/*Repository*.java: '트랜잭션 밖 Lazy 로딩 문제와 영속성 컨텍스트 오염 가능성을 점검한다.'🤖 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/product/ProductJpaRepository.java` around lines 36 - 38, softDeleteByIds performs a JPQL bulk update that bypasses the persistence context and can leave stale Product entities in the first-level cache; update the ProductJpaRepository.softDeleteByIds declaration to either add `@Modifying`(clearAutomatically = true, flushAutomatically = true) or ensure callers invoke EntityManager.clear() immediately after the method to evict cached Product instances, and add an integration test that loads a Product via findById(), calls softDeleteByIds(List.of(id)), then reads the Product again in the same transaction and asserts deletedAt is set to verify the change is visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47ffe412-cf3c-424f-8e26-ba31621ade7a
⛔ Files ignored due to path filters (4)
.claude/rules/project/architecture.mdis excluded by!**/*.mdand included by**docs/performance/order/index-analysis.mdis excluded by!**/*.mdand included by**docs/performance/product/cache-strategy.mdis excluded by!**/*.mdand included by**docs/performance/product/index-analysis.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (47)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeService.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderService.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductScheduler.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/domain/order/Order.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderStatus.javaapps/commerce-api/src/main/java/com/loopers/domain/product/Product.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheInvalidationConfig.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheManager.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/CursorResponse.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentRequest.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/OrderExperimentV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentController.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentRequest.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/experiment/ProductExperimentV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminApiV1Spec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderRequest.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/transaction/TransactionHelper.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductCacheManagerIntegrationTest.javadocs/performance/order/explain-analysis.sqldocs/performance/order/seed-orders.sqldocs/performance/product/explain-analysis.sqldocs/performance/product/seed-products.sqlk6/detail-test.jsk6/list-test.jsmodules/redis/src/main/java/com/loopers/config/redis/RedisConfig.javamodules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java
💤 Files with no reviewable changes (1)
- modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java
Comment on lines
+247
to
+251
| Map<Long, Brand> brandMap = brandService.getBrandsMapByIds(brandIds); | ||
|
|
||
| List<ProductInfo> infoList = content.stream() | ||
| .map(product -> ProductInfo.from(product, brandMap.get(product.getBrandId()).getName())) | ||
| .toList(); |
There was a problem hiding this comment.
커서 목록 변환 경로에서 브랜드 누락 시 NPE가 발생한다
운영 관점에서 Line 250의 brandMap.get(...).getName()는 데이터 불일치 시 500(NPE)로 떨어져 장애 분석이 어려워지고, 오류 응답 포맷 일관성도 깨진다.
수정안으로 toProductInfoPage(Line 229-233)와 동일하게 사전 검증 후 CoreException(ErrorType.NOT_FOUND, ...)를 던지도록 맞추는 것이 안전하다.
추가 테스트로는 “브랜드 맵 누락 상황에서 커서 API가 NPE가 아닌 CoreException으로 실패하는지”를 반드시 검증해야 한다.
수정 예시 diff
private CursorResult toCursorResult(List<Product> products, int size) {
@@
Map<Long, Brand> brandMap = brandService.getBrandsMapByIds(brandIds);
+ for (Product product : content) {
+ if (!brandMap.containsKey(product.getBrandId())) {
+ throw new CoreException(ErrorType.NOT_FOUND,
+ "브랜드 매핑 누락. productId=" + product.getId() + ", brandId=" + product.getBrandId());
+ }
+ }
+
List<ProductInfo> infoList = content.stream()
.map(product -> ProductInfo.from(product, brandMap.get(product.getBrandId()).getName()))
.toList();Based on learnings: CoreException을 통해 ApiControllerAdvice로 일관된 오류 응답을 유지해야 한다는 규칙을 적용했다.
🤖 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/product/ProductExperimentFacade.java`
around lines 247 - 251, In ProductExperimentFacade, avoid the NPE caused by
brandMap.get(...).getName() when mapping content to ProductInfo: before calling
ProductInfo.from (the mapping at the code using brandMap and ProductInfo.from),
validate that brandMap contains product.getBrandId() and if missing throw new
CoreException(ErrorType.NOT_FOUND, "brand", product.getBrandId()) (match the
behavior in toProductInfoPage), so the cursor endpoint fails with a
CoreException handled by ApiControllerAdvice rather than a NullPointerException;
also add a unit/integration test asserting the cursor API returns CoreException
(NOT_FOUND) when a product's brand id is absent from brandMap.
Comment on lines
+27
to
+35
| while (true) { | ||
| List<Long> ids = productService.findIdsForCleanup(brandId, BATCH_SIZE); | ||
| if (ids.isEmpty()) break; | ||
|
|
||
| int deleted = productService.softDeleteByIds(ids); | ||
| totalDeleted += deleted; | ||
| } while (deleted == BATCH_SIZE); | ||
|
|
||
| if (ids.size() < BATCH_SIZE) break; | ||
| Thread.sleep(100); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ProductScheduler.java" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 172
🏁 Script executed:
find . -type f -name "ProductService.java" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
find . -type f -name "*.java" -path "*product*" | grep -E "(ProductScheduler|ProductService)" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 359
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/application/product/ProductScheduler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1916
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/application/product/ProductService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6151
🏁 Script executed:
find . -type f -name "ProductRepository.java" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
rg -t java "findIdsByBrandIdForCleanup|softDeleteByIds" --context 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6422
🏁 Script executed:
rg -t java "SELECT.*FOR UPDATE|SKIP LOCKED|Redisson|Lock|Mutex" --context 2 | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1526
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1595
🏁 Script executed:
find . -type f -name "Product.java" -path "*/domain/product/*" | head -3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 158
🏁 Script executed:
cat -n ./apps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceIntegrationTest.java | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4719
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2645
🏁 Script executed:
find . -type f -name "ProductJpaRepository.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 179
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6149
🏁 Script executed:
rg -t java "ProductScheduler|cleanup" apps/commerce-api/src/test/ --context 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
배치 ID 조회와 삭제가 서로 다른 트랜잭션이라 중복 실행에 취약하다.
현재 cleanup()에는 트랜잭션이 없고 findIdsForCleanup()와 softDeleteByIds()가 각각 커밋되므로, 두 노드가 같은 brandId를 동시에 처리하면 같은 ID 배치를 함께 가져올 수 있다. 운영 환경에서는 불필요한 쓰기와 로그 왜곡이 발생하며, softDeleteByIds()에 deleted_at IS NULL 조건이 없으면 이미 삭제된 상품의 deletedAt도 재기록된다.
배치 선점과 삭제를 다시 원자적으로 묶거나, SELECT id FROM products WHERE brand_id = :brandId AND deleted_at IS NULL ORDER BY id FOR UPDATE SKIP LOCKED LIMIT :batchSize로 선점한 뒤 softDeleteByIds()에 AND deleted_at IS NULL 조건을 추가하도록 수정해 달라. 동일 브랜드를 두 워커가 동시에 정리해도 각 상품이 한 번만 삭제 표시되는 동시성 테스트를 추가해 달라.
또한 InterruptedException 발생 시 현재는 재인터럽트 후 다음 브랜드 정리를 계속하는데, 스케줄된 정리 작업이 중단되었을 때 남은 브랜드를 부분 처리하지 않도록 루프를 즉시 종료해야 한다.
🤖 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/product/ProductScheduler.java`
around lines 27 - 35, Make cleanup() atomic and resilient to concurrent workers
by changing the ID-selection and delete behavior: wrap the select-and-delete in
a single transaction (use a transactional boundary around the work in
ProductScheduler.cleanup()), change findIdsForCleanup(...) to select and lock
rows for preemption (SELECT id FROM products WHERE brand_id = :brandId AND
deleted_at IS NULL ORDER BY id FOR UPDATE SKIP LOCKED LIMIT :batchSize) so IDs
are reserved, and make softDeleteByIds(...) include AND deleted_at IS NULL to
avoid re-writing deletedAt; keep the selection and softDeleteByIds(...) call
inside the same transaction to prevent duplicate processing. Add a concurrency
test asserting two workers on the same brand only mark each product deleted
once. Finally, on InterruptedException from Thread.sleep(...) stop the outer
brand loop immediately (do not continue to next brand) and preserve the thread's
interrupted status.
Comment on lines
+41
to
+43
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ProductScheduler.java" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 172
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/application/product/ProductScheduler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1916
🏁 Script executed:
find . -type f -name "*ProductSchedulerTest*" -o -name "*ProductScheduler*Test*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
find . -type f -path "*/test/*" -name "*ProductScheduler*" -o -path "*/test/*" -name "*Product*Scheduler*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
find ./apps/commerce-api -type f -name "*Test.java" | grep -i schedulerRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
find ./apps/commerce-api -type d -name "test"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 113
🏁 Script executed:
ls -la ./apps/commerce-api/src/test/java/com/loopers/application/product/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 283
🏁 Script executed:
find ./apps/commerce-api/src/test -name "*Scheduler*Test*" -o -name "*Scheduler*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
grep -r "ProductScheduler" ./apps/commerce-api/src/test --include="*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
cat -n ./apps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceIntegrationTest.java | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2045
인터럽트 발생 시 남은 브랜드 처리를 즉시 중단해야 한다.
현재 코드는 interrupt() 복원 후 계속 진행하므로, 다음 반복의 sleep(100)이 즉시 InterruptedException을 던진다. 이로 인해 남은 모든 브랜드가 연쇄적으로 스킵되어 운영 환경에서 종료·재배포 시 정리 작업이 부분적으로 수행되고 끝난다. 인터럽트 복원 후 즉시 메서드를 return으로 빠져나가고, 원인 추적을 위해 예외를 로그에 포함해야 한다.
수정 예시
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
- log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId);
+ log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId, e);
+ return;
} catch (Exception e) {인터럽트 발생 시 findIdsForCleanup()이 호출되지 않는 단위 테스트를 추가해야 한다.
📝 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
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId); | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId, e); | |
| return; |
🤖 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/product/ProductScheduler.java`
around lines 41 - 43, In ProductScheduler.catch block for InterruptedException
(in class ProductScheduler, method that loops brands), restore the interrupt as
currently done, then immediately return from the method so processing of
remaining brands stops; include the caught exception object in the log call
(e.g., log.warn("브랜드 {} 상품 정리 중 인터럽트 발생", brandId, e)) so the stacktrace is
recorded; after this change add a unit test asserting that when
InterruptedException occurs, findIdsForCleanup() is not called for remaining
brands.
Comment on lines
+41
to
+50
| public void putList(Long brandId, String sort, int page, int size, CachedPage cachedPage) { | ||
| try { | ||
| String key = listKey(brandId, sort, page, size); | ||
| String json = objectMapper.writeValueAsString(cachedPage); | ||
| redisTemplate.opsForValue().set(key, json, LIST_TTL); | ||
| redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key); | ||
| } catch (JsonProcessingException e) { | ||
| log.warn("상품 목록 캐시 저장 실패. brandId={}, sort={}", brandId, sort, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
LIST_KEYS_REGISTRY가 무한히 증가할 수 있다 - TTL 또는 정리 로직이 필요하다.
putList()에서 캐시 키를 LIST_KEYS_REGISTRY Set에 추가하지만, 개별 캐시 엔트리가 5분 TTL로 만료되어도 레지스트리 항목은 삭제되지 않는다. 장기 운영 시 레지스트리 Set이 무한히 커질 수 있으며, evictAllLists()에서 이미 만료된 키들에 대해 불필요한 삭제 요청이 발생한다.
🐛 해결 방안
방안 1: 레지스트리에 TTL 설정 (권장)
public void putList(Long brandId, String sort, int page, int size, CachedPage cachedPage) {
try {
String key = listKey(brandId, sort, page, size);
String json = objectMapper.writeValueAsString(cachedPage);
redisTemplate.opsForValue().set(key, json, LIST_TTL);
redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key);
+ // 레지스트리 TTL을 LIST_TTL보다 약간 길게 설정
+ redisTemplate.expire(LIST_KEYS_REGISTRY, LIST_TTL.plusMinutes(1));
} catch (JsonProcessingException e) {
log.warn("상품 목록 캐시 저장 실패. brandId={}, sort={}", brandId, sort, e);
}
}방안 2: evictAllLists()에서 레지스트리만 삭제하고 키는 TTL 만료에 의존
목록 캐시 무효화 시 레지스트리만 삭제하고, 개별 키는 TTL로 자연 만료되도록 처리한다.
📝 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
| public void putList(Long brandId, String sort, int page, int size, CachedPage cachedPage) { | |
| try { | |
| String key = listKey(brandId, sort, page, size); | |
| String json = objectMapper.writeValueAsString(cachedPage); | |
| redisTemplate.opsForValue().set(key, json, LIST_TTL); | |
| redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key); | |
| } catch (JsonProcessingException e) { | |
| log.warn("상품 목록 캐시 저장 실패. brandId={}, sort={}", brandId, sort, e); | |
| } | |
| } | |
| public void putList(Long brandId, String sort, int page, int size, CachedPage cachedPage) { | |
| try { | |
| String key = listKey(brandId, sort, page, size); | |
| String json = objectMapper.writeValueAsString(cachedPage); | |
| redisTemplate.opsForValue().set(key, json, LIST_TTL); | |
| redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key); | |
| // 레지스트리 TTL을 LIST_TTL보다 약간 길게 설정 | |
| redisTemplate.expire(LIST_KEYS_REGISTRY, LIST_TTL.plusMinutes(1)); | |
| } catch (JsonProcessingException e) { | |
| log.warn("상품 목록 캐시 저장 실패. brandId={}, sort={}", brandId, sort, e); | |
| } | |
| } |
🤖 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/product/ProductCacheManager.java`
around lines 41 - 50, The LIST_KEYS_REGISTRY set is never trimmed so it can grow
forever; in putList(Long brandId, String sort, int page, int size, CachedPage
cachedPage) after adding the key with
redisTemplate.opsForSet().add(LIST_KEYS_REGISTRY, key) ensure the registry
itself has a TTL (e.g., call redisTemplate.expire(LIST_KEYS_REGISTRY,
LIST_KEYS_TTL)) or explicitly set a TTL on the registry key so stale entries
auto-clear; alternatively (simpler) change evictAllLists() to only delete the
registry key (redisTemplate.delete(LIST_KEYS_REGISTRY)) and rely on LIST_TTL for
individual entries instead of iterating and deleting each key—pick one approach
and update putList, evictAllLists, and add a constant LIST_KEYS_TTL if choosing
the TTL-on-registry option.
Comment on lines
+6
to
+11
| public record CursorResponse<T>( | ||
| List<T> content, | ||
| Long nextCursor, | ||
| boolean hasNext, | ||
| int size | ||
| ) { |
There was a problem hiding this comment.
응답 레코드의 불변성이 보장되지 않아 운영 중 응답 일관성이 깨질 수 있다
Line 6-11은 content를 그대로 보관하므로 호출자가 원본 리스트를 변경하면 직렬화 결과가 바뀔 수 있어 장애 재현과 원인 분석이 어려워진다. 컴팩트 생성자에서 방어적 복사와 불변식 검증을 추가하는 수정이 필요하다. 추가 테스트로 (1) 생성 후 원본 리스트 변경 시 content()가 변하지 않는지, (2) content == null, size < 0 입력 시 예외가 발생하는지를 검증해야 한다.
수정 예시
package com.loopers.interfaces.api;
import java.util.List;
import java.util.function.Function;
public record CursorResponse<T>(
List<T> content,
Long nextCursor,
boolean hasNext,
int size
) {
+ public CursorResponse {
+ if (content == null) {
+ throw new IllegalArgumentException("content must not be null");
+ }
+ if (size < 0) {
+ throw new IllegalArgumentException("size must be non-negative");
+ }
+ content = List.copyOf(content);
+ }
+
public static <T> CursorResponse<T> of(List<T> content, Long nextCursor, boolean hasNext, int size) {
return new CursorResponse<>(content, nextCursor, hasNext, size);
}As per coding guidelines **/*.java: null 처리, 방어적 복사, 불변성 점검을 수행해야 한다.
🤖 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/interfaces/api/CursorResponse.java`
around lines 6 - 11, The CursorResponse record currently stores the provided
mutable `content` list directly; add a compact constructor for
`CursorResponse<T>` that validates `content` is non-null and `size` is >= 0, and
assigns an immutable defensive copy (e.g., List.copyOf(content)) to the
`content` component so `content()` cannot change if the caller mutates the
original list; throw a NullPointerException for null `content` and an
IllegalArgumentException for `size < 0`. Also add unit tests that (1) mutate the
original list after constructing CursorResponse and assert `content()` is
unchanged, and (2) assert exceptions are thrown for `content == null` and for
negative `size`.
Comment on lines
+12
to
+19
| public static <T> CursorResponse<T> of(List<T> content, Long nextCursor, boolean hasNext, int size) { | ||
| return new CursorResponse<>(content, nextCursor, hasNext, size); | ||
| } | ||
|
|
||
| public static <S, T> CursorResponse<T> of(List<S> content, Long nextCursor, boolean hasNext, int size, Function<S, T> mapper) { | ||
| List<T> mapped = content.stream().map(mapper).toList(); | ||
| return new CursorResponse<>(mapped, nextCursor, hasNext, size); | ||
| } |
There was a problem hiding this comment.
size를 외부에서 주입받는 구조는 커서 메타데이터 오염 위험이 크다
Line 12와 Line 16에서 size를 인자로 받으면 실제 content.size()와 불일치한 응답이 생성되어 페이지네이션 판단 오류와 캐시 오염으로 이어질 수 있다. size는 팩토리 내부에서 계산하도록 시그니처를 단순화하고, Line 16의 mapper/content는 즉시 null 검증하는 수정이 안전하다. 추가 테스트로 (1) 매핑 팩토리 결과의 size가 항상 mapped.size()와 일치하는지, (2) mapper == null일 때 명시적 예외 메시지가 발생하는지를 검증해야 한다.
수정 예시
+import java.util.Objects;
+
- public static <T> CursorResponse<T> of(List<T> content, Long nextCursor, boolean hasNext, int size) {
- return new CursorResponse<>(content, nextCursor, hasNext, size);
+ public static <T> CursorResponse<T> of(List<T> content, Long nextCursor, boolean hasNext) {
+ List<T> copied = List.copyOf(content);
+ return new CursorResponse<>(copied, nextCursor, hasNext, copied.size());
}
- public static <S, T> CursorResponse<T> of(List<S> content, Long nextCursor, boolean hasNext, int size, Function<S, T> mapper) {
- List<T> mapped = content.stream().map(mapper).toList();
- return new CursorResponse<>(mapped, nextCursor, hasNext, size);
+ public static <S, T> CursorResponse<T> of(List<S> content, Long nextCursor, boolean hasNext, Function<S, T> mapper) {
+ Objects.requireNonNull(content, "content must not be null");
+ Objects.requireNonNull(mapper, "mapper must not be null");
+ List<T> mapped = content.stream().map(mapper).toList();
+ return new CursorResponse<>(mapped, nextCursor, hasNext, mapped.size());
}🤖 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/interfaces/api/CursorResponse.java`
around lines 12 - 19, The two CursorResponse.of factory methods accept an
external size which can diverge from content.size(); change both factory
signatures so they no longer accept a size and instead compute size = content ==
null ? 0 : content.size() internally; in the generic mapper overload
(CursorResponse.of(List<S> content, Long nextCursor, boolean hasNext,
Function<S,T> mapper)) add immediate null checks for content and mapper and
throw IllegalArgumentException with a clear message if either is null, map the
content to List<T> mapped = content.stream().map(mapper).toList(), compute size
= mapped.size(), and return new CursorResponse<>(mapped, nextCursor, hasNext,
size); update the non-mapper overload to compute size = content.size()
similarly; add tests to assert that returned.getSize() equals the actual list
size and that passing a null mapper throws the expected exception.
Comment on lines
+20
to
+35
| public record UserOrderList( | ||
| @NotNull(message = "userId는 필수입니다") | ||
| Long userId, | ||
|
|
||
| OrderStatus status, | ||
|
|
||
| LocalDate startDate, | ||
|
|
||
| LocalDate endDate, | ||
|
|
||
| @PositiveOrZero(message = "페이지 번호는 0 이상이어야 합니다") | ||
| Integer page, | ||
|
|
||
| @Min(value = 1, message = "페이지 크기는 1 이상이어야 합니다") | ||
| @Max(value = 100, message = "페이지 크기는 100 이하여야 합니다") | ||
| Integer size |
There was a problem hiding this comment.
Bean Validation 어노테이션 사용은 프로젝트 컨벤션 위반이다.
프로젝트 정책에 따르면 DTO에 Bean Validation 어노테이션(@NotNull, @PositiveOrZero, @Min, @Max 등)을 사용하지 않고, 검증 로직은 도메인 레이어에서 처리해야 한다. 현재 방식은 MethodArgumentNotValidException을 발생시켜 CoreException 기반 통합 에러 핸들링 패턴과 충돌한다.
문제점(운영 관점):
- 에러 응답 포맷이
ApiControllerAdvice의CoreException핸들링과 불일치하여 클라이언트가 두 가지 에러 포맷을 처리해야 한다. - 검증 실패 시 발생하는 예외 타입이 달라 모니터링/알람 설정이 복잡해진다.
수정안: 어노테이션을 제거하고, 서비스/도메인 레이어에서 검증 후 CoreException을 던지도록 변경한다.
추가 테스트: 유효하지 않은 요청에 대해 CoreException 기반 에러 응답이 반환되는지 통합 테스트로 검증해야 한다.
Based on learnings: "Do not use Bean Validation annotations on DTOs in this project. Move validation logic into the domain layer" 및 "enforce unified error handling by routing errors through CoreException to ApiControllerAdvice".
🤖 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/interfaces/api/experiment/OrderExperimentRequest.java`
around lines 20 - 35, Remove all Bean Validation annotations from the
UserOrderList record (remove `@NotNull` on userId, `@PositiveOrZero` on page,
`@Min/`@Max on size, and any others) and keep only the plain fields (userId,
status, startDate, endDate, page, size); then move the validation logic into the
service or domain layer (e.g., in the method that consumes UserOrderList) to
validate userId presence, page >= 0, and 1 <= size <= 100 and throw the
project’s CoreException with appropriate error codes/messages when validations
fail so ApiControllerAdvice handles errors uniformly; finally add/adjust an
integration test to assert invalid requests produce the CoreException-based
error response.
Comment on lines
+6
to
+7
| const HOT_PRODUCT_IDS = Array.from({ length: 100 }, (_, i) => i + 1); | ||
| const COLD_PRODUCT_IDS = Array.from({ length: 9900 }, (_, i) => i + 101); |
There was a problem hiding this comment.
상품 ID 범위 하드코딩은 환경별 재현성을 떨어뜨린다.
운영 관점에서 데이터셋이 달라지면 404 비율이 급증해 성능 실험 결과가 부정확해진다. 수정안으로 PRODUCT_MAX_ID, HOT_PRODUCT_COUNT를 __ENV로 주입하고, 범위 유효성 검증(최소/최대)을 추가하는 것이 필요하다.
수정 예시
-const HOT_PRODUCT_IDS = Array.from({ length: 100 }, (_, i) => i + 1);
-const COLD_PRODUCT_IDS = Array.from({ length: 9900 }, (_, i) => i + 101);
+const PRODUCT_MAX_ID = Number(__ENV.PRODUCT_MAX_ID || 10000);
+const HOT_PRODUCT_COUNT = Number(__ENV.HOT_PRODUCT_COUNT || 100);
+const HOT_COUNT = Math.min(Math.max(HOT_PRODUCT_COUNT, 1), PRODUCT_MAX_ID);
+const COLD_COUNT = Math.max(PRODUCT_MAX_ID - HOT_COUNT, 0);
+
+const HOT_PRODUCT_IDS = Array.from({ length: HOT_COUNT }, (_, i) => i + 1);
+const COLD_PRODUCT_IDS = Array.from({ length: COLD_COUNT }, (_, i) => i + HOT_COUNT + 1);추가 테스트로 PRODUCT_MAX_ID=500, HOT_PRODUCT_COUNT=50 환경에서 생성되는 ID가 항상 1..500 범위 내인지 확인해야 한다.
Also applies to: 71-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k6/detail-test.js` around lines 6 - 7, Replace the hardcoded HOT_PRODUCT_IDS
and COLD_PRODUCT_IDS generation with environment-driven ranges: read
PRODUCT_MAX_ID and HOT_PRODUCT_COUNT from __ENV (with sensible defaults),
validate they are integers and that 1 <= HOT_PRODUCT_COUNT <= PRODUCT_MAX_ID and
PRODUCT_MAX_ID >= 1, then generate HOT_PRODUCT_IDS as 1..HOT_PRODUCT_COUNT and
COLD_PRODUCT_IDS as (HOT_PRODUCT_COUNT+1)..PRODUCT_MAX_ID (or empty if equal).
Update any helper that uses HOT_PRODUCT_IDS/COLD_PRODUCT_IDS (references in this
file around lines creating those arrays) to rely on these validated values so
tests with PRODUCT_MAX_ID=500 and HOT_PRODUCT_COUNT=50 produce IDs only in
1..500.
Comment on lines
+93
to
+99
| export function testV1() { | ||
| const productId = getProductId(); | ||
| const res = http.get(`${BASE_URL}/api/experiment/products/v1/${productId}`, params); | ||
| check(res, { 'v1 status 200': (r) => r.status === 200 }); | ||
| v1Duration.add(res.timings.duration); | ||
| v1Requests.add(1); | ||
| } |
There was a problem hiding this comment.
실패 응답을 성공 지연시간 지표에 합산하면 성능 결론이 왜곡된다.
운영 관점에서 check() 실패(4xx/5xx) 요청도 v*_detail_duration에 포함되어 버전별 성능 비교가 오염된다. 수정안으로 성공(200)일 때만 Trend/요청 카운터를 기록하고, 실패는 별도 에러 카운터로 분리하는 것이 안전하다.
수정 예시
+const v1Errors = new Counter('v1_detail_errors');
+const v2Errors = new Counter('v2_detail_errors');
+const v3Errors = new Counter('v3_detail_errors');
+
+function recordDetail(res, checkName, durationMetric, requestMetric, errorMetric) {
+ const ok = check(res, { [checkName]: (r) => r.status === 200 });
+ if (ok) {
+ durationMetric.add(res.timings.duration);
+ requestMetric.add(1);
+ } else {
+ errorMetric.add(1);
+ }
+}
+
export function testV1() {
const productId = getProductId();
const res = http.get(`${BASE_URL}/api/experiment/products/v1/${productId}`, params);
- check(res, { 'v1 status 200': (r) => r.status === 200 });
- v1Duration.add(res.timings.duration);
- v1Requests.add(1);
+ recordDetail(res, 'v1 status 200', v1Duration, v1Requests, v1Errors);
}추가 테스트로 존재하지 않는 productId를 의도적으로 주입했을 때 v*_detail_duration 샘플 수는 증가하지 않고 v*_detail_errors만 증가하는지 검증해야 한다.
Also applies to: 101-107, 109-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k6/detail-test.js` around lines 93 - 99, The testV1 function currently
records duration and request count for all responses, which skews performance
metrics when checks fail; update testV1 so that after the http.get and check()
you only call v1Duration.add(...) and v1Requests.add(1) when res.status === 200,
and increment a separate error counter (e.g., v1Errors.add(1)) for non-200
responses; also add/initialize the v1Errors metric if missing and apply the same
pattern to the equivalent blocks mentioned (lines 101-107 and 109-115) so that
only successful responses contribute to v*_detail_duration and
v*_detail_requests while failures increment v*_detail_errors.
Comment on lines
+145
to
+150
| export function testOffsetV1() { | ||
| const page = getPage(); | ||
| const res = http.get(`${BASE_URL}/api/experiment/products/v1/offset?page=${page}&size=20`); | ||
| check(res, { 'v1 offset 200': (r) => r.status === 200 }); | ||
| v1OffsetDuration.add(res.timings.duration); | ||
| } |
There was a problem hiding this comment.
목록 성능 지표도 실패 응답과 분리해 기록해야 한다.
운영 관점에서 실패 요청이 지연시간 Trend에 섞이면 Offset/Cursor 및 버전 간 비교 정확도가 깨진다. 수정안으로 성공 응답만 v*_offset_duration/v*_cursor_duration에 적재하고, 실패는 별도 에러 카운터로 분리해야 한다.
수정 예시
+const v1OffsetErrors = new Counter('v1_offset_errors');
+function recordIfOk(res, checkName, durationMetric, errorMetric) {
+ const ok = check(res, { [checkName]: (r) => r.status === 200 });
+ if (ok) durationMetric.add(res.timings.duration);
+ else errorMetric.add(1);
+}
+
export function testOffsetV1() {
const page = getPage();
const res = http.get(`${BASE_URL}/api/experiment/products/v1/offset?page=${page}&size=20`);
- check(res, { 'v1 offset 200': (r) => r.status === 200 });
- v1OffsetDuration.add(res.timings.duration);
+ recordIfOk(res, 'v1 offset 200', v1OffsetDuration, v1OffsetErrors);
}추가 테스트로 잘못된 쿼리 파라미터를 주입했을 때 duration Trend 샘플은 증가하지 않고 error Counter만 증가하는지 확인해야 한다.
Also applies to: 152-157, 159-164, 168-173, 175-180, 182-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k6/list-test.js` around lines 145 - 150, In testOffsetV1, only record the
duration metric (v1OffsetDuration.add(...)) when the response is successful
(res.status === 200); for non-200 responses increment a separate error counter
(e.g., v1OffsetErrorCounter.add(1)) instead of recording duration. Implement the
same pattern in the other list test functions referenced (the offset/cursor
variants and other versions): guard each duration metric (v*_offset_duration /
v*_cursor_duration) behind a success check and create/use per-endpoint error
counters (v*_offset_error_counter / v*_cursor_error_counter) to count failures.
Ensure you reuse the existing check(...) call for assertions but move metric
updates into the success/failure branches in functions like testOffsetV1 and the
corresponding testOffset/testCursor functions mentioned.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.java (1)
58-70:evictDetail/evictAllLists에서 Redis 삭제 실패 시 예외 처리가 없다.Redis 연결 문제나 타임아웃 발생 시 예외가 전파되어 트랜잭션 롤백을 유발할 수 있다.
afterCommit콜백에서 호출되는 경우 예외가 로그 없이 무시될 수 있어 캐시 무효화 실패를 인지하기 어렵다.🛡️ 수정안
public void evictDetail(Long productId) { + try { redisTemplate.delete(detailKey(productId)); + } catch (Exception e) { + log.warn("상품 상세 캐시 삭제 실패. productId={}", productId, e); + } publishInvalidation("detail:" + productId); } public void evictAllLists() { + try { Set<String> keys = redisTemplate.opsForSet().members(LIST_KEYS_REGISTRY); if (keys != null && !keys.isEmpty()) { redisTemplate.delete(keys); } redisTemplate.delete(LIST_KEYS_REGISTRY); + } catch (Exception e) { + log.warn("상품 목록 캐시 삭제 실패", e); + } publishInvalidation("list:all"); }추가 테스트: Redis 연결 실패 시 evict 메서드가 예외를 전파하지 않고 로그만 남기는지 검증이 필요하다.
🤖 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/product/ProductCacheManager.java` around lines 58 - 70, evictDetail and evictAllLists currently perform redisTemplate.delete and publishInvalidation without handling Redis failures, which can propagate exceptions and break transactions or be silently dropped in afterCommit; wrap the Redis operations and the publishInvalidation call in try/catch blocks around redisTemplate.delete(detailKey(productId)), redisTemplate.opsForSet().members(LIST_KEYS_REGISTRY), redisTemplate.delete(keys), redisTemplate.delete(LIST_KEYS_REGISTRY) and publishInvalidation("detail:"+productId)/publishInvalidation("list:all") to catch runtime exceptions (e.g., RedisConnectionFailureException), log a clear error via your logger including the productId or LIST_KEYS_REGISTRY context and the exception, and do not rethrow so callers (and afterCommit callbacks) won’t fail; keep behavior otherwise unchanged.apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.java (1)
173-174:CachedPage.totalElements를hasNext플래그로 오용하고 있다.
CachedPage는 offset 페이징용으로 설계되어totalElements가 전체 개수를 의미하는데, cursor 페이징에서hasNextboolean (0 또는 1)을 저장하는 용도로 사용하고 있다. 운영 관점에서 캐시 데이터 디버깅 시totalElements=1이 "다음 페이지 존재"를 의미한다는 것을 파악하기 어렵다.♻️ 수정안: cursor용 별도 캐시 record 정의
// CachedCursorPage record 추가 (ProductCacheManager 내부) public record CachedCursorPage(List<ProductInfo> content, Long nextCursor, boolean hasNext) {}또는 현재 구조를 유지할 경우,
CachedPage사용 시 cursor 페이징에서의 필드 의미를 주석으로 명시해야 한다.🤖 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/product/ProductExperimentFacade.java` around lines 173 - 174, The code is misusing CachedPage.totalElements to store a cursor/hasNext flag; add a proper cursor-specific cache record and store that instead: define a new record (e.g., CachedCursorPage(List<ProductInfo> content, Long nextCursor, boolean hasNext)) inside ProductCacheManager (or alongside CachedPage), then change the productCacheManager.putList call that currently passes new CachedPage(...) to create and persist a CachedCursorPage with result.content(), the actual next cursor (or null), and result.hasNext(); alternatively, if you cannot change the cache shape now, add a clear comment at the productCacheManager.putList call and on CachedPage explaining that totalElements is being repurposed for cursor hasNext to avoid confusion during ops/debugging.
🤖 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/infrastructure/product/ProductCacheManager.java`:
- Around line 98-112: The getList method currently increments listHit before
attempting to deserialize and then increments listMiss again on
JsonProcessingException, causing both counters to increase on failure; update
getList (referencing getList, listHit, listMiss, objectMapper.readValue,
CachedPage) so that listHit.incrementAndGet() is called only after
objectMapper.readValue(json, CachedPage.class) succeeds (i.e., move the hit
increment into the try after successful read and return Optional.of(...)), and
ensure on exception only listMiss.incrementAndGet() is invoked (remove or avoid
any hit increment in the catch).
- Around line 82-96: getDetail currently increments detailHit before
deserialization, so if objectMapper.readValue(json, ProductInfo.class) throws
the code incorrectly counts both a hit and a miss; change the logic so
detailHit.incrementAndGet() happens only after successful readValue (i.e., after
you obtain the ProductInfo and before returning Optional.of(...)), keep
detailMiss.incrementAndGet() on null and in the catch block only, and add a unit
test for getDetail to assert that on JsonProcessingException detailHit is not
incremented while detailMiss is.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.java`:
- Around line 173-174: The code is misusing CachedPage.totalElements to store a
cursor/hasNext flag; add a proper cursor-specific cache record and store that
instead: define a new record (e.g., CachedCursorPage(List<ProductInfo> content,
Long nextCursor, boolean hasNext)) inside ProductCacheManager (or alongside
CachedPage), then change the productCacheManager.putList call that currently
passes new CachedPage(...) to create and persist a CachedCursorPage with
result.content(), the actual next cursor (or null), and result.hasNext();
alternatively, if you cannot change the cache shape now, add a clear comment at
the productCacheManager.putList call and on CachedPage explaining that
totalElements is being repurposed for cursor hasNext to avoid confusion during
ops/debugging.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.java`:
- Around line 58-70: evictDetail and evictAllLists currently perform
redisTemplate.delete and publishInvalidation without handling Redis failures,
which can propagate exceptions and break transactions or be silently dropped in
afterCommit; wrap the Redis operations and the publishInvalidation call in
try/catch blocks around redisTemplate.delete(detailKey(productId)),
redisTemplate.opsForSet().members(LIST_KEYS_REGISTRY),
redisTemplate.delete(keys), redisTemplate.delete(LIST_KEYS_REGISTRY) and
publishInvalidation("detail:"+productId)/publishInvalidation("list:all") to
catch runtime exceptions (e.g., RedisConnectionFailureException), log a clear
error via your logger including the productId or LIST_KEYS_REGISTRY context and
the exception, and do not rethrow so callers (and afterCommit callbacks) won’t
fail; keep behavior otherwise unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8beb3530-8190-4947-a763-37a8f00e8bef
⛔ Files ignored due to path filters (1)
docs/performance/product/cache-strategy.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (3)
apps/commerce-api/src/main/java/com/loopers/application/product/ProductExperimentFacade.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductCacheManager.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheManager.java
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheManager.java
Comment on lines
+82
to
+96
| public Optional<ProductInfo> getDetail(Long productId) { | ||
| String json = redisTemplate.opsForValue().get(detailKey(productId)); | ||
| if (json == null) { | ||
| detailMiss.incrementAndGet(); | ||
| return Optional.empty(); | ||
| } | ||
| try { | ||
| detailHit.incrementAndGet(); | ||
| return Optional.of(objectMapper.readValue(json, ProductInfo.class)); | ||
| } catch (JsonProcessingException e) { | ||
| log.warn("상품 상세 캐시 역직렬화 실패. productId={}", productId, e); | ||
| detailMiss.incrementAndGet(); | ||
| return Optional.empty(); | ||
| } | ||
| } |
There was a problem hiding this comment.
캐시 히트/미스 통계 카운트 로직에 오류가 있다.
역직렬화 실패 시 detailHit(Line 89)과 detailMiss(Line 93)가 모두 증가하여 통계가 왜곡된다. 운영 관점에서 캐시 히트율 모니터링 시 정확하지 않은 지표로 잘못된 튜닝 결정을 유발할 수 있다.
🐛 수정안
public Optional<ProductInfo> getDetail(Long productId) {
String json = redisTemplate.opsForValue().get(detailKey(productId));
if (json == null) {
detailMiss.incrementAndGet();
return Optional.empty();
}
try {
- detailHit.incrementAndGet();
- return Optional.of(objectMapper.readValue(json, ProductInfo.class));
+ ProductInfo info = objectMapper.readValue(json, ProductInfo.class);
+ detailHit.incrementAndGet();
+ return Optional.of(info);
} catch (JsonProcessingException e) {
log.warn("상품 상세 캐시 역직렬화 실패. productId={}", productId, e);
detailMiss.incrementAndGet();
return Optional.empty();
}
}추가 테스트: 역직렬화 실패 시나리오에서 hit 카운트가 증가하지 않고 miss만 증가하는지 검증하는 단위 테스트가 필요하다.
🤖 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/product/ProductCacheManager.java`
around lines 82 - 96, getDetail currently increments detailHit before
deserialization, so if objectMapper.readValue(json, ProductInfo.class) throws
the code incorrectly counts both a hit and a miss; change the logic so
detailHit.incrementAndGet() happens only after successful readValue (i.e., after
you obtain the ProductInfo and before returning Optional.of(...)), keep
detailMiss.incrementAndGet() on null and in the catch block only, and add a unit
test for getDetail to assert that on JsonProcessingException detailHit is not
incremented while detailMiss is.
Comment on lines
+98
to
+112
| public Optional<CachedPage> getList(Long brandId, String sort, int page, int size) { | ||
| String json = redisTemplate.opsForValue().get(listKey(brandId, sort, page, size)); | ||
| if (json == null) { | ||
| listMiss.incrementAndGet(); | ||
| return Optional.empty(); | ||
| } | ||
| try { | ||
| listHit.incrementAndGet(); | ||
| return Optional.of(objectMapper.readValue(json, CachedPage.class)); | ||
| } catch (JsonProcessingException e) { | ||
| log.warn("상품 목록 캐시 역직렬화 실패. brandId={}, sort={}", brandId, sort, e); | ||
| listMiss.incrementAndGet(); | ||
| return Optional.empty(); | ||
| } | ||
| } |
There was a problem hiding this comment.
getList에도 동일한 히트/미스 카운트 오류가 존재한다.
getDetail과 동일하게 역직렬화 실패 시 listHit(Line 105)과 listMiss(Line 109)가 모두 증가한다.
🐛 수정안
public Optional<CachedPage> getList(Long brandId, String sort, int page, int size) {
String json = redisTemplate.opsForValue().get(listKey(brandId, sort, page, size));
if (json == null) {
listMiss.incrementAndGet();
return Optional.empty();
}
try {
- listHit.incrementAndGet();
- return Optional.of(objectMapper.readValue(json, CachedPage.class));
+ CachedPage cachedPage = objectMapper.readValue(json, CachedPage.class);
+ listHit.incrementAndGet();
+ return Optional.of(cachedPage);
} catch (JsonProcessingException e) {
log.warn("상품 목록 캐시 역직렬화 실패. brandId={}, sort={}", brandId, sort, e);
listMiss.incrementAndGet();
return Optional.empty();
}
}🤖 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/product/ProductCacheManager.java`
around lines 98 - 112, The getList method currently increments listHit before
attempting to deserialize and then increments listMiss again on
JsonProcessingException, causing both counters to increase on failure; update
getList (referencing getList, listHit, listMiss, objectMapper.readValue,
CachedPage) so that listHit.incrementAndGet() is called only after
objectMapper.readValue(json, CachedPage.class) succeeds (i.e., move the hit
increment into the try after successful read and return Optional.of(...)), and
ensure on exception only listMiss.incrementAndGet() is invoked (remove or avoid
any hit increment in the catch).
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
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
🧭 Context & Decision
최적화 전략 — 3단계 순차 적용
핵심 의사결정
1. 인덱스: 핵심 + 방어 이중 안전망
(deleted_at, brand_id, sort_column)2. 주문 인덱스: 데이터 분포가 성능을 좌우
3. 캐시: v1/v2/v3 단계별 독립 측정
📊 성능 측정 결과
상품 인덱스 EXPLAIN Before/After
주문 인덱스 EXPLAIN Before/After
k6 부하 테스트 (20 VUs, 순차 실행)
상품 목록 — Offset Pagination
상품 목록 — Cursor Pagination
🏗️ 변경 범위
인덱스
(deleted_at, brand_id, created_at DESC)(deleted_at, brand_id, price)(deleted_at, brand_id, like_count DESC)(deleted_at, created_at DESC)(deleted_at, like_count DESC)(deleted_at, price)(user_id, status, created_at DESC)(user_id, created_at DESC)(status, created_at DESC)(product_id, order_id)캐시
주요 코드 변경
📚 분석 문서
✅ 체크리스트
🔖 Index
❤️ Structure
⚡ Cache
📊 추가 과제
💬 리뷰 포인트
1. 방어 인덱스 전략의 적정선
상품에 핵심 3개 + 방어 3개 = 6개 인덱스 설정.
여기서 "방어 인덱스"란, 브랜드 필터 없는 전체 조회에서 캐시 미스가 발생했을 때 filesort를 방지하기 위해 추가한 인덱스(
(deleted_at, like_count DESC)등).캐시가 살아있으면 DB를 안 타지만, TTL 만료·Redis 장애·무효화 직후에 49,787건 filesort가 발생하는 것을 막기 위한 안전망.
"캐시 미스 시에도 DB가 견딜 수 있어야 한다"는 판단인데, 실무에서 인덱스 6개가 쓰기 비용 대비 과한 수준인지 멘토님의 경험이 궁금합니다.
2. 좋아요 변경 시 목록 캐시 무효화 범위
현재 좋아요 등록/취소 시 상세 캐시만 evict하고, 목록은 TTL(5분) 의존입니다. 좋아요 빈도가 높아 전체 목록 evict 시 캐시 적중률이 급락한다고 판단했는데, 실무에서는 좋아요순 목록만 선택적 evict하는 방식이 더 일반적인지 궁금합니다.
3. 프로덕션 L1(Caffeine) 전환 시점
실험 코드에서 v3(Caffeine+Redis)의 p99가 v2 대비 82% 개선됨을 확인했지만, Pub/Sub 유실 가능성 때문에 프로덕션에는 v2(Redis만)를 적용했습니다. Pub/Sub 안정성을 어느 수준까지 검증하면 L1 전환이 적절한지, 혹은 TTL 안전망만으로 충분히 프로덕션에 올릴 수 있는지 멘토님의 판단이 궁금합니다.