Skip to content

[Volume-7] ApplicationEvent 경계 분리 + Kafka Outbox Pipeline + 선착순 쿠폰 - 임나현#274

Open
iohyeon wants to merge 20 commits intoLoopers-dev-lab:iohyeonfrom
iohyeon:volume-7
Open

[Volume-7] ApplicationEvent 경계 분리 + Kafka Outbox Pipeline + 선착순 쿠폰 - 임나현#274
iohyeon wants to merge 20 commits intoLoopers-dev-lab:iohyeonfrom
iohyeon:volume-7

Conversation

@iohyeon
Copy link
Copy Markdown

@iohyeon iohyeon commented Mar 26, 2026

📌 Summary

  • 배경: 주문·좋아요·쿠폰의 부가 로직이 핵심 TX에 결합되어 있다. 좋아요 집계 DB 장애 → 주문 롤백. 포인트 서비스 지연 → 주문 응답 지연. 핵심과 부가의 실패 영향도가 같을 이유가 없다.
  • 목표: 핵심 트랜잭션 우선, 부가 로직은 eventually. ApplicationEvent로 JVM 내 경계를 분리하고, Kafka + Outbox Pattern으로 시스템 간 이벤트를 신뢰성 있게 전달한다. 선착순 쿠폰은 Kafka 파티션 순차 처리로 락 없이 정합성을 보장한다.
  • 결과: ApplicationEvent 경계 분리, Outbox + Kafka 파이프라인, 선착순 쿠폰 구현 완료. 메시지 유실 가능 구간 10개를 식별하여 방어 로직 구현. 선착순 쿠폰 100명/200명 동시 요청에서 초과 발급 0건 확인. Graceful Shutdown, PROCESSING 복구, Prometheus 메트릭까지 구현 완료.

선행 실험

본 PR의 주요 설계 결정은 별도 랩 프로젝트에서 먼저 실험한 것입니다.

  • kafka-pipeline-lab — Outbox Relay 비동기 콜백 버그 발견, event_handled 멱등성 검증, 선착순 쿠폰 1000명 동시 테스트. "동기 .get() 전환 이유", "DB 멱등성 선택 이유"의 실험
  • kafka-eda-lab — 코레오그래피/오케스트레이션 Saga 비교 + 장애 시뮬레이션(오프셋 미커밋, 이벤트 체인 끊김). "ApplicationEvent로 충분한 것 vs Kafka가 필요한 것"의 경계 판단과 신뢰성 설계에 반영됐습니다.

🧭 Context & Decision

이번 PR의 설계는 초기 구현의 제약사항을 발견하고 개선하는 과정에서 도출되었습니다.
각 설계 결정은 구현 단계에서 직면한 구체적인 문제와 그 해결 과정을 반영합니다.

핵심 결정 요약

# 결정 항목 최종 선택 핵심 근거
1 경계 분리 기준 "실패 시 주문도 실패해야 하는가?" 부가 로직(좋아요, 포인트) 실패가 주문 롤백을 유발하는 구조적 결함 제거
2 Outbox 저장 방식 Facade TX 안에서 직접 저장 AFTER_COMMIT(TX 밖 실행)과 BEFORE_COMMIT(의미적 모순) 모두 실패 → 직접 호출
3 Relay 발행 방식 동기 .get(10s) 비동기 콜백은 별도 스레드 실행 → 상태 변경이 saveAll() 이후로 밀림
4 파이프라인 신뢰성 Producer 멱등성 + Consumer 멱등성 + DLQ 3가지 취약점 발견 → 수정 후 유실 구간 10개 방어
5 Consumer 예외 전략 Consumer/Processor 분리 + Spring ErrorHandler self-invocation 방지 + 배치 블로킹(무한 리밸런싱) 방지
6 선착순 쿠폰 Kafka 파티션 순차 처리 같은 key → 같은 파티션 → 단일 Consumer 순차 → 동시 접근 불가
7 Relay 처리량 2-Phase + FOR UPDATE SKIP LOCKED 처리량 50배 개선(10→500건/초 이론) + 멀티 인스턴스 중복 방지
8 운영 안정성 Graceful Shutdown + PROCESSING 복구 + 메트릭 데이터 유실 방지, 장애 자동 복구, 실시간 관측성

1. 경계 분리 — "이 로직이 실패하면, 주문도 실패해야 하는가?"

이 질문을 기준으로 로직을 분류했습니다.

로직 실패 시 주문도 실패? 결정
재고 예약 Yes — 가용 재고 확보가 주문의 전제 같은 TX (단, "예약"이지 "차감"이 아님)
좋아요 집계 No — 집계 늦어도 주문은 유효 이벤트 분리
포인트 적립 No — 나중에 적립해도 됨 이벤트 분리
캐시 무효화 No — TTL이 안전망 이벤트 분리

재고를 "같은 TX"에 넣되, **차감이 아니라 예약(reserve)**으로 설계했습니다. TX1에서 reservedQty를 올려 가용 재고를 확보하고, PG 결제 후 TX2에서 commit()으로 실제 quantity를 차감합니다. 실패하면 release()로 예약을 해제합니다. 이렇게 하면 PG 호출 동안 비관적 락을 잡고 있지 않아도 다른 주문이 정확한 가용 재고를 계산할 수 있습니다.

TX1: reserve(qty)    → reservedQty 증가, quantity 유지  (가용재고 = quantity - reservedQty)
PG 결제: 락 미보유 상태에서 외부 호출
TX2 성공: commit(qty) → quantity 차감 + reservedQty 차감
TX2 실패: release(qty) → reservedQty 복원 (보상 트랜잭션)

🔗 Inventory.java#L98 — reserve()
🔗 Inventory.java#L112 — commit()
🔗 Inventory.java#L120 — release()
🔗 OrderFacade.java#L202 — TX1에서 reserveAll()
🔗 OrderFacade.java#L251 — TX2에서 commitAll()

재고 확정(commit) 자체를 이벤트로 분리하는 것도 가능합니다 — 주문 생성 시 예약만 하고, 결제 승인 후 "재고 확정" 이벤트를 발행하는 구조입니다. 현재 구현에서는 TX2에서 동기적으로 commit하는 방식을 선택했지만, 결제 흐름이 이미 "진행 중" 상태를 표시하는 구조이므로 이벤트 기반 확정으로의 전환 여지는 열어두고 있습니다.

ApplicationEvent + @TransactionalEventListener(AFTER_COMMIT)으로 부가 로직을 TX 밖으로 분리했습니다. 주문 커밋이 확정된 후에만 실행되므로, 부가 로직 실패가 주문에 영향을 주지 않습니다.

여기까지는 순조로웠습니다.


2. Outbox 도입 — 세 차례의 설계 개선

commerce-streamer(별도 앱)에서 상품 메트릭을 집계해야 합니다. ApplicationEvent는 같은 JVM에서만 동작하므로 Kafka가 필요합니다. 하지만 DB 커밋과 Kafka 발행을 동시에 보장할 수 없습니다(Dual Write Problem). Outbox Pattern으로 해결합니다 — 여기까지는 교과서적입니다.

문제는 "Outbox를 어떻게 저장하느냐"였습니다.

첫 번째 시도: AFTER_COMMIT 리스너에서 Outbox 저장

TX 커밋 → AFTER_COMMIT 리스너 → outboxEventService.save()

깨진 이유: OrderFacade에서 TransactionTemplate.execute()를 사용하는데, execute() 반환 시점에 TX가 이미 커밋된 상태입니다. AFTER_COMMIT 리스너가 바인딩할 트랜잭션이 없어서 아예 실행되지 않았습니다.

그보다 근본적인 문제 — AFTER_COMMIT과 Outbox의 트랜잭션 요구사항이 정반대입니다:

AFTER_COMMIT: TX 커밋 '후'에 실행
Outbox 원자성: TX '안'에서 저장해야 함

두 번째 시도: BEFORE_COMMIT으로 전환

BEFORE_COMMIT이면 TX 안에서 실행됩니다. 원자성은 보장됩니다.

깨진 이유 두 가지:

  1. OrderConfirmedEvent가 커밋 전에 처리됨 — "확정됐다"는 이벤트가 확정 전에 실행되는 의미적 모순
  2. 프로젝트 전체가 AFTER_COMMIT 기반(캐시 무효화, 로깅)인데 Outbox만 BEFORE_COMMIT — 같은 이벤트 추상화 안에서 phase가 혼재

최종 결정: 직접 저장

// LikeFacade.java#L52-L65
@Transactional
public LikeResult likeProduct(Long userId, Long productId) {
    Product product = productService.getDisplayableProduct(productId);
    likeService.like(userId, productId);
    productService.incrementLikeCount(productId);
    productCacheManager.registerDetailOnlyEvictAfterCommit(productId);

    // Outbox 저장 — 같은 TX (좋아요 집계 → catalog-events-v1)
    outboxEventService.save("PRODUCT", productId,
            "ProductLikedEvent", new ProductLikedEvent(userId, productId, true),
            "catalog-events-v1", String.valueOf(productId));

    return new LikeResult(product.getLikeCount() + 1);
}

🔗 LikeFacade.java#L52-L65

ApplicationEvent를 경유하지 않습니다. 두 도구는 서로 다른 문제를 풉니다:

ApplicationEvent: 같은 JVM, 부가 처리 (캐시, 로깅) — 유실 허용
Outbox:           시스템 간, 신뢰성 전달 (Kafka) — 유실 불허

이 분리 방식은 반복적인 설계 개선을 거쳐 도출되었습니다. 초기 구현의 제약사항을 직접 경험하며 도달한 결론입니다.


3. Relay — 왜 비동기 콜백이 아닌 동기 .get()인가

Outbox에 쌓인 PENDING 이벤트를 Kafka에 보내는 Relay입니다. 1초 Polling → 500건 조회 → 발행 → markPublished.

비동기 whenComplete로 구현하면 markPublished()가 영속되지 않는 문제가 발생합니다.

// ❌ 비동기 콜백의 문제
kafkaTemplate.send(...)
    .whenComplete((result, ex) -> {
        event.markPublished();  // 별도 스레드에서 실행 → saveAll() 이후에 호출될 수 있음
    });
// markPublished()가 saveAll()에 반영되지 않음 → 무한 재발행

원인: whenComplete 콜백은 Kafka 응답이 돌아올 때 별도 스레드에서 실행됩니다. markPublished() 호출 시점이 상태 저장(saveAll()) 이후로 밀리면, 상태 변경이 누락되어 같은 이벤트가 다음 폴링에서 다시 발행됩니다. Outbox 저장 방식에서 겪었던 AFTER_COMMIT 문제와 본질이 같습니다 — 상태 변경과 저장의 타이밍이 어긋나면 안 된다.

// ✅ 동기 — 발행 결과 확정 후 상태 변경, saveAll() 전에 보장 (OutboxRelayService.java#L222-L229)
SendResult<Object, Object> result = kafkaTemplate
    .send(producerRecord)
    .get(10, TimeUnit.SECONDS);
event.markPublished();

🔗 OutboxRelayService.java#L103-L161 — publishProcessingEvents (Phase 2)
🔗 OutboxRelayService.java#L211-L245 — publishToKafka (.get() 동기 발행)

Outbox 패턴 선택 기준: "작고 단순하면 Polling, 크고 빠르고 중요하면 CDC." 이 원칙에 따르면 "처리량이 문제가 되면 CDC로 전환하는 것이 하이브리드 방식보다 단순하고 명확합니다."

방식 정합성 처리량 비고
Polling (sync) ← 현재 높음 이론 500건/초, 실측 ~155건/초 안전하고 단순. 실측 병목은 Phase 2의 동기 Kafka I/O
Polling (async) ⚠️ 버그 - TX 밖 콜백 → 무한 재발행
CDC (Debezium) 높음 매우 높음 (실시간 수준) 처리량이 병목이 되면 전환 대상

4. 신뢰성 — 파이프라인 취약점 분석

Outbox + Relay + Consumer 파이프라인을 구축한 뒤, 메시지 유실 가능 구간을 분석했습니다. 파이프라인 전체를 검토한 결과 3가지 취약점을 발견했습니다.

취약점 1: Producer 멱등성 미설정

Relay → Broker 저장 ✅ → ACK 전송 중 네트워크 장애 → Relay는 실패로 인식 → 재발행 → 같은 메시지 2건.

enable.idempotence=true가 설정되어 있지 않았습니다. PID + Sequence 기반 중복 제거가 동작하지 않는 상태였습니다.

취약점 2: CatalogMetricsConsumer 멱등성 부재

incrementLikeCount()는 상대 연산(+1)이라 멱등하지 않습니다. 같은 이벤트가 재배달되면 like_count가 2 증가합니다.

CouponIssueConsumer에는 event_handled 체크가 있었지만 CatalogMetricsConsumer에는 없었습니다. 같은 파이프라인인데 한쪽만 방어된 비대칭.

취약점 3: 배치 처리 중 실패 레코드 유실

배치 10건: 16 성공 → 7 실패(예외) → 810 성공 → ack.acknowledge(). 7번 레코드는 Kafka 관점에서 "처리됨"이지만 실제로는 유실. At-Most-Once.

3건 모두 수정 완료:

구멍 수정 파일
Producer 멱등성 enable.idempotence=true, acks=all kafka.yml#L17-L20
Consumer 멱등성 event_handled + 같은 @transactional CatalogMetricsProcessor.java
배치 유실 실패 레코드 → DLQ 동기 격리 DlqPublisher.java#L66

수정 후 주요 유실 구간 방어:

# 구간 방어
1 TX 롤백 ✅ Outbox도 같은 TX → 함께 롤백
2 Relay 프로세스 죽음 ✅ PENDING 유지 → 다음 Relay가 이어서
3 Kafka 발행 중 네트워크 장애 ✅ .get() 타임아웃 → PENDING 유지 → 재시도
4 ACK 유실 → 중복 발행 ✅ Producer idempotence (PID+Seq)
5 markPublished() 전에 Relay 죽음 ✅ Consumer event_handled
6 Broker 다운 ✅ Relay 재시도 (최대 5회)
7 Consumer 처리 후 ACK 전 장애 ✅ event_handled (같은 TX)
8 배치 중 일부 실패 ✅ DLQ 격리
9 정상 처리
10 ACK 전송 실패 → 재배달 ✅ event_handled

5. Consumer 예외 전략 — Consumer/Processor 분리 + Spring ErrorHandler 위임

두 가지 설계 결정이 있었습니다.

self-invocation 문제 해결: Consumer → Processor 분리

초기에는 Consumer 내부에서 @Transactional 메서드를 호출했지만, 같은 빈 내부 호출(self-invocation)은 Spring AOP 프록시를 통과하지 않아 @Transactional이 무시됩니다. event_handled 저장과 비즈니스 로직이 같은 TX가 아니게 되면 멱등성 보장이 깨집니다.

해결: 처리 로직을 별도 @Service Bean으로 분리했습니다.

CouponIssueConsumer (@Component, Interfaces)
  → CouponIssueProcessor (@Service, Application, @Transactional)

CatalogMetricsConsumer (@Component, Interfaces)
  → CatalogMetricsProcessor (@Service, Application, @Transactional)

Consumer는 Kafka 메시지 수신 + 예외 분류만 담당하고, Processor가 @Transactional 안에서 비즈니스 로직 + event_handled 저장을 원자적으로 처리합니다.

🔗 CouponIssueProcessor.java@transactional 비즈니스 로직
🔗 CatalogMetricsProcessor.java@transactional 메트릭 집계

재시도 전략 진화: Consumer 내부 재시도 → Spring ErrorHandler 위임

초기에는 Consumer 내부에서 3회 재시도 + exponential backoff(1초→2초→4초)를 구현했습니다. 하지만 배치 리스너에서 7초 백오프가 전체 배치를 블로킹하는 문제를 발견했습니다. 최악의 경우 500건 × 7초 = 3,500초 → MAX_POLL_INTERVAL_MS(120초) 초과 → 무한 리밸런싱 루프.

해결: Consumer 내부 재시도를 제거하고 Spring DefaultErrorHandler + DeadLetterPublishingRecoverer에 위임했습니다.

// KafkaConfig.java — ErrorHandler 설정
private CommonErrorHandler createErrorHandler(KafkaTemplate<Object, Object> kafkaTemplate) {
    DeadLetterPublishingRecoverer recoverer =
        new DeadLetterPublishingRecoverer(kafkaTemplate,
            (record, ex) -> new TopicPartition("pipeline-dlq-v1", -1));
    return new DefaultErrorHandler(recoverer, new FixedBackOff(0L, 0L));  // 즉시 DLQ
}

현재 에러 흐름:

예외 처리 이유
BusinessFailureException (재고 소진, 중복 발급) Consumer에서 catch → FAILED 기록 → ACK Processor가 이미 DB에 상태 기록. 재시도 무의미.
인프라 예외 (DB 타임아웃 등) Consumer에서 throw → Spring ErrorHandler → 즉시 DLQ 배치 블로킹 방지. DLQ에서 원인 분석 후 수동 재처리.

CatalogMetricsConsumer도 동일 구조: 모든 예외가 인프라 장애이므로 Spring ErrorHandler가 즉시 DLQ로 격리합니다.


6. 선착순 쿠폰 — 락 대신 Kafka 파티션

key=couponTemplateId → murmur2(key) % partitions → 같은 파티션 → 단일 Consumer 순차 처리. 동시 접근 자체가 불가능합니다.

같은 쿠폰 → 같은 파티션 → 순차 (정합성)
다른 쿠폰 → 다른 파티션 → 병렬 (처리량)

Facade에서 요청을 저장하고 requestId를 반환합니다. 사용자는 polling으로 결과를 확인하는 구조입니다.

POST /coupons/issue → { requestId: "abc", status: "PENDING" }
GET  /coupons/issue/abc → { status: "PENDING" }
GET  /coupons/issue/abc → { status: "ISSUED" }

🔗 CouponFacade.java#L56-L73 — requestCouponIssue (Outbox + 요청 저장)

멱등성: EventHandledEntity.java (UNIQUE event_id) + 같은 TX. Redis SET NX가 아닌 이유는 — 비즈니스 실패 시 Redis에 "처리됨"이 남아 재시도가 불가능하기 때문입니다. DB + 같은 TX면 비즈니스 실패 시 함께 롤백되어 재시도 가능합니다.

왜 직접 Kafka 발행이 아니라 Outbox를 경유했는가: 선착순 쿠폰 요청은 "발급해 달라"는 Command 성격이므로, 유실 시 사용자가 재요청하면 복구됩니다. 이 관점에서는 Outbox 없이 직접 Kafka 발행(+ 실패 시 에러 응답 → 재요청)도 선택지입니다. 그러나 시스템 간 이벤트 전파를 Outbox 단일 경로로 통일하는 것을 우선했습니다. 경로가 하나면 장애 분석 시 "Outbox 상태만 보면 된다"는 운영 단순성이 확보되고, 선착순에만 별도 발행 경로를 두면 장애 시 확인해야 할 지점이 2개로 늘어납니다. 처리량 병목(RP03)은 인지하고 있으며, 즉시성이 요구되는 규모로 확장되면 직접 발행 + Outbox fallback 하이브리드 또는 CDC 전환을 검토할 수 있습니다.


7. PROCESSING 상태 추가 — 처리량과 중복 방지

Relay 구현 후 처리량을 검토했습니다. 5초마다 50건 조회 → 최대 10건/초. 선착순 쿠폰이나 재고 소진 이벤트가 몰릴 때는 병목입니다.

문제는 "폴링 간격만 줄이면 되나?"였습니다.

발견한 두 가지 문제

  1. 처리량 병목: 5초 폴링 + 50건 배치 → 최대 10건/초
  2. 멀티 인스턴스 중복 발행 위험: 같은 PENDING 이벤트를 여러 인스턴스가 동시에 조회하면 중복 발행

단순히 간격을 1초로 줄이면 처리량은 50건/초가 되지만, 멀티 인스턴스 환경에서 같은 이벤트를 중복으로 Kafka에 발행할 수 있습니다.

해결: PROCESSING 상태 + FOR UPDATE SKIP LOCKED

2-Phase Relay로 해결했습니다:

// OutboxRelayService.java#L74-L94
@Scheduled(fixedDelay = 1000)  // 1초마다
@Transactional
public void markPendingAsProcessing() {
    // Phase 1: PENDING → PROCESSING (FOR UPDATE SKIP LOCKED)
    List<OutboxEventEntity> pending =
        outboxEventJpaRepository.findPendingEventsForUpdate(BATCH_SIZE);

    pending.forEach(OutboxEventEntity::markProcessing);
    outboxEventJpaRepository.saveAll(pending);
}

@Scheduled(fixedDelay = 1000)  // 1초마다
public void publishProcessingEvents() {
    // Phase 2: PROCESSING → Kafka 발행 (병렬)
    List<OutboxEventEntity> processing =
        outboxEventJpaRepository.findProcessingEvents(BATCH_SIZE);

    // Partition Key별 그룹화 → 병렬 발행
    var grouped = processing.stream()
        .collect(groupingBy(OutboxEventEntity::getPartitionKey));

    grouped.values().parallelStream().forEach(events -> {
        for (OutboxEventEntity event : events) {
            publishToKafka(event);
        }
    });
}

🔗 OutboxRelayService.java#L74-L94 — markPendingAsProcessing (Phase 1)
🔗 OutboxRelayService.java#L103-L161 — publishProcessingEvents (Phase 2)

-- OutboxEventJpaRepository.java (Native Query)
SELECT * FROM outbox_event
WHERE status = 'PENDING'
ORDER BY created_at ASC
LIMIT :limit
FOR UPDATE SKIP LOCKED

🔗 OutboxEventJpaRepository.java#L22-L29 — FOR UPDATE SKIP LOCKED

// OutboxStatus.java
public enum OutboxStatus {
    PENDING,      // 발행 대기
    PROCESSING,   // 발행 중 (멀티 인스턴스 중복 방지)
    PUBLISHED,    // 발행 완료
    FAILED        // 발행 실패
}

🔗 OutboxStatus.java#L3-L8

동작 원리:

  1. Phase 1 (상태 변경): FOR UPDATE SKIP LOCKED로 PENDING 이벤트 조회 → 락 획득한 행만 즉시 PROCESSING으로 변경 → 다른 인스턴스는 락 대기 없이 건너뜀(SKIP LOCKED)
  2. Phase 2 (Kafka 발행): PROCESSING 상태의 이벤트들을 partition key별로 그룹화하여 병렬 발행 → 성공 시 PUBLISHED, 실패 시 FAILED

멀티 인스턴스 환경:

Instance A: PENDING #1-500 조회 → FOR UPDATE 락 획득 → PROCESSING 변경
Instance B: PENDING 조회 → #1-500은 이미 락됨 → SKIP LOCKED → #501-1000 조회

같은 이벤트를 동시에 처리할 수 없습니다. 상태 변경이 먼저, Kafka 발행이 나중입니다.

개선 결과

항목 변경 전 변경 후 (이론) 실측값 비고
폴링 간격 5초 1초
배치 크기 50건 500건
발행 지연 평균 2.5초 (최대 5초) 평균 0.5초 (최대 1초) 5x 단축 — 모든 부하에서 체감
Phase 1 (500건) < 500ms 62~165ms DB 상태 변경만. 빠름
Phase 2 (500건) < 500ms 1.8~4.6초 동기 .get(10s) + parallelStream. 실제 병목
1000건 처리량 500건/초 84.5~95.3건/초 Phase 2의 Kafka I/O가 병목
5000건 burst 소화 10초 (10사이클) 32.337초, 135155건/초 500건/사이클 × 10사이클

실측 환경: 2회 측정 (Testcontainers MySQL + Docker MySQL). Docker Kafka 단일 브로커. 이론값(500건/초)과 실측값(95155건/초)의 괴리는 Phase 2의 동기 .get() Kafka I/O가 배치당 2.43.8초를 소비하기 때문이다. Phase 1은 내부 측정 62~165ms이나, E2E(DB 커넥션 + TX 커밋 포함) 기준 ~1.5초. 처리량 개선은 Phase 2 최적화(비동기 전환 또는 CDC)로 가능하다. CDC 전환 시점: 이벤트 생성 속도가 ~135건/초를 지속적으로 초과하면 Polling이 따라잡지 못함.

지연시간과 처리량은 다른 지표입니다. 폴링 간격 축소(5초→1초)는 모든 부하 수준에서 발행 지연을 줄이고, 배치 크기 확대(50→500)는 부하가 높을 때 병목을 방지합니다. 현재 트래픽이 낮다면 50x 처리량 개선은 체감되지 않지만, 지연시간 5x 단축은 이벤트 1건이어도 체감됩니다.

고민: 즉시 발행 + 보완재 폴링

Polling의 구조적 한계는 "이벤트 생성 → 다음 폴링까지 대기"입니다. 1초로 줄여도 최대 1초 지연은 남아있습니다.

이를 해결하는 방법으로 TX 커밋 직후 즉시 Kafka 발행을 시도하고, 실패 시 Outbox + 폴링이 수거하는 하이브리드 구조를 검토했습니다:

TX1: 비즈니스 로직 + Outbox 저장 → 커밋
TX 커밋 직후: afterCommit에서 kafkaTemplate.send() → whenComplete 콜백에서 새 TX로 상태 변경
실패 시: PENDING 유지 → 기존 폴링이 1분~5분 뒤 수거 (보완재)

구현하지 않은 이유:

  • whenComplete 콜백이 Kafka I/O 스레드에서 실행되므로 TransactionTemplate으로 새 TX를 명시적으로 열어야 하고, 이 과정에서의 실패 시나리오가 추가됨
  • 발행 경로가 2개(즉시 + 폴링)가 되어 디버깅 복잡도가 올라감
  • 현재 1초 폴링으로 대부분의 유스케이스에서 충분한 지연 수준

다만 지연에 더 민감한 도메인이 생기면 전환할 수 있는 구조입니다. Outbox 테이블과 PROCESSING 상태 머신이 이미 있으므로, 즉시 발행을 추가해도 폴링이 안전망으로 동작합니다.

🔗 OutboxRelayService.java#L45 — BATCH_SIZE = 500, fixedDelay = 1000

PROCESSING 상태로 선점하는 구조를 잡아두면, 인스턴스를 늘려도 Relay 코드 변경 없이 처리량만 선형으로 증가합니다. 멀티 인스턴스 중복 발행은 스케일아웃 시 반드시 마주치는 문제이므로, 확장 시점에 Relay를 재설계하는 것보다 처음부터 구조적으로 해결하는 것이 비용이 낮다고 판단했습니다.


8. Outbox 안정성 개선

PROCESSING 상태 추가 후, 운영 환경을 고려한 안정성 개선을 진행했습니다.

개선 사항 (6가지)

1. updated_at 컬럼 추가

@Column(name = "updated_at", nullable = false)
private ZonedDateTime updatedAt;

@PreUpdate
protected void onUpdate() {
    updatedAt = ZonedDateTime.now();
}
  • PROCESSING 지속 시간 추적 가능
  • 오래된 PROCESSING 이벤트 감지 및 복구

2. Graceful Shutdown 구현

@PreDestroy
public void onShutdown() {
    shuttingDown.set(true);
    // Phase 2 완료 대기 (최대 30초)
    currentLatch.await(30, TimeUnit.SECONDS);
    // 미완료 PROCESSING → PENDING 복원
    recoverProcessingOnShutdown();
}
  • 앱 종료 시 진행 중인 작업 완료 대기
  • 미완료 이벤트 PENDING 복원
  • 데이터 유실 방지 ✅

3. 명시적 PROCESSING 복구 로직

@Scheduled(fixedDelay = 300000) // 5분
@Transactional
public void recoverStalledProcessingEvents() {
    ZonedDateTime threshold = ZonedDateTime.now().minusMinutes(5);
    List<OutboxEventEntity> stalled =
        repository.findStalledProcessingEvents(threshold);

    stalled.forEach(OutboxEventEntity::markRetry);
    repository.saveAll(stalled);
}
  • Phase 2 실패 시 자동 복구
  • 앱 크래시 후 재시작 시 복구
  • 5분 주기 안전망

4. DB 인덱스 최적화

-- MySQL 호환 복합 인덱스 (Partial Index는 PostgreSQL 전용이므로 복합 인덱스로 대체)
CREATE INDEX idx_outbox_status_updated ON outbox_event (status, updated_at);
CREATE INDEX idx_outbox_status_created ON outbox_event (status, created_at);
  • status를 선두 컬럼에 배치하여 PENDING/PROCESSING 조회 모두 인덱스 활용
  • 초기에는 PostgreSQL Partial Index로 작성했으나, 프로젝트 DB가 MySQL이라 복합 인덱스로 교체

5. parallelStream 예외 처리 강화

grouped.values().parallelStream().forEach(events -> {
    for (OutboxEventEntity event : events) {
        try {
            publishToKafka(event);
        } catch (Exception e) {
            // 개별 이벤트 실패해도 다른 이벤트 계속 처리
            event.markFailed("Unexpected error: " + e.getMessage());
            metrics.recordPublishFailure();
        }
    }
});
  • 한 이벤트 실패해도 다른 이벤트 정상 처리
  • Phase 2 안정성 향상

6. Prometheus 모니터링 메트릭

outbox_pending_count: PENDING 이벤트 수
outbox_processing_count: PROCESSING 이벤트 수
outbox_oldest_pending_age_seconds: 가장 오래된 PENDING 나이
outbox_phase1_duration_seconds: Phase 1 처리 시간
outbox_phase2_duration_seconds: Phase 2 처리 시간
outbox_publish_success_total: 발행 성공 횟수
outbox_publish_failed_total: 발행 실패 횟수

🏗️ Design Overview

이벤트 발행 방식 — 세 가지 경로

이벤트 경로 토픽 Key 이유
ProductLiked/UnlikedEvent Outbox → Relay → Kafka catalog-events-v1 productId 시스템 간 + 정합성 필요
OrderItemSoldEvent Outbox → Relay → Kafka catalog-events-v1 productId 시스템 간 + 정합성 필요
CouponIssueRequestedEvent Outbox → Relay → Kafka coupon-issue-requests-v1 couponTemplateId 시스템 간 + 순서 보장
OrderConfirmedEvent Outbox → Relay → Kafka (예정) order-events-v1 orderId 포인트 적립 등 후속 처리
ProductViewedEvent ApplicationEvent - - 같은 JVM + 유실 허용
UserActivityEvent 토픽 정의 완료, 발행 미구현 user-activity-events-v1 (예정) userId 추후 직접 Kafka 발행 예정 (유실 허용, Outbox 불필요)

주요 컴포넌트 책임

컴포넌트 책임
OutboxEventService Facade TX 안에서 이벤트를 outbox_event에 저장
OutboxRelayService 1초 Polling → 2-Phase Relay (PENDING→PROCESSING→Kafka) → 복구 로직 (5분)
CatalogMetricsConsumer catalog-events-v1 → product_metrics upsert. 멱등성: event_handled
CouponIssueConsumerCouponIssueProcessor coupon-issue-requests-v1 → 쿠폰 발급. Consumer/Processor 분리(self-invocation 해결). 비즈니스 예외는 catch, 인프라 예외는 Spring ErrorHandler → DLQ
CatalogMetricsConsumerCatalogMetricsProcessor Consumer/Processor 분리. @transactional Processor 안에서 메트릭 upsert + event_handled 원자적 저장
DlqPublisher + Spring DefaultErrorHandler 인프라 예외 → Spring ErrorHandler → DeadLetterPublishingRecoverer → pipeline-dlq-v1. 재시도 0회 (배치 블로킹 방지)
OutboxMetrics Prometheus 메트릭 9개 수집 (Gauge/Timer/Counter)

변경 범위

구분 파일 수 내용
이벤트 인프라 10개 Outbox Entity/Service/Relay + EventHandled/Idempotency + DlqPublisher(api+streamer) + TopicConfig + OutboxMetrics
Producer (Facade) 4개 Like/Order/Coupon/Product Facade에서 Outbox 또는 ApplicationEvent 발행
Consumer + Processor 4개 CatalogMetricsConsumer/Processor (streamer) + CouponIssueConsumer/Processor (api) — self-invocation 해결
이벤트/설정 다수 이벤트 클래스, kafka.yml, KafkaTopicConfig (5개 토픽), 이벤트 리스너 삭제 등
테스트 8개 (29건) Kafka동시성(3) + Consumer(3) + Facade비동기(3) + EventHandled(3) + Outbox(7) + OutboxMetrics(7) + GracefulShutdown(3)

🔁 Flow Diagram

핵심 흐름 — 2-Phase Outbox Relay

sequenceDiagram
    autonumber
    participant User
    participant Facade
    participant DB as DB (outbox_event)
    participant Relay as OutboxRelay
    participant Kafka
    participant Consumer
    participant Metrics as product_metrics

    User->>Facade: POST /likes
    activate Facade
    Note over Facade,DB: @Transactional 시작
    Facade->>DB: 좋아요 저장 + likeCount++
    Facade->>DB: outbox_event (status=PENDING)
    Note over Facade,DB: TX 커밋 (원자성 보장)
    Facade-->>User: 200 OK (즉시 응답)
    deactivate Facade

    Note over Relay: 1초마다 Polling

    rect rgb(230, 240, 255)
        Note over Relay,DB: Phase 1: 상태 변경 (빠름)
        Relay->>DB: SELECT FOR UPDATE SKIP LOCKED
        Relay->>DB: status = PROCESSING
        Note over Relay: 락 빠르게 해제
    end

    rect rgb(255, 240, 230)
        Note over Relay,Kafka: Phase 2: Kafka 발행 (병렬)
        Relay->>DB: SELECT PROCESSING

        par Partition Key 별 병렬 발행
            Relay->>Kafka: send().get(10s) - ProductId #1
        and
            Relay->>Kafka: send().get(10s) - ProductId #2
        and
            Relay->>Kafka: send().get(10s) - ProductId #3
        end

        Kafka-->>Relay: ACK (동기 대기)
        Relay->>DB: status = PUBLISHED
    end

    Kafka->>Consumer: ProductLikedEvent

    rect rgb(240, 255, 240)
        Note over Consumer,Metrics: Consumer 처리 (멱등성)
        Consumer->>DB: event_handled 존재 확인
        alt 신규 이벤트
            Consumer->>Metrics: incrementLikeCount()
            Consumer->>DB: event_handled 저장
            Note over Consumer: 같은 TX (원자성)
        else 중복 이벤트
            Note over Consumer: 스킵 (멱등성 보장)
        end
    end

    Consumer->>Kafka: ACK
Loading

선착순 쿠폰 — 파티션 순차 처리

sequenceDiagram
    autonumber
    participant Users as 100명 동시 요청
    participant API as CouponFacade
    participant DB as DB (outbox + request)
    participant Relay as OutboxRelay
    participant Kafka as Kafka<br/>(partition 0)
    participant Consumer as CouponIssue<br/>Consumer

    Users->>API: POST /coupons/issue (동시 100건)

    loop 100건 요청
        API->>DB: request(PENDING) + outbox(PENDING)
        API-->>Users: 202 Accepted {requestId}
    end

    Note over Relay,Kafka: Relay가 순차적으로 Kafka 발행

    Relay->>DB: SELECT FOR UPDATE SKIP LOCKED
    Relay->>DB: status = PROCESSING (100건)

    loop 100건 발행
        Relay->>Kafka: CouponIssueRequestedEvent
        Note over Kafka: key=couponTemplateId<br/>→ 모두 같은 파티션
    end

    Note over Consumer: 단일 파티션 → 순차 처리 보장

    loop 100건 순차 처리
        Kafka->>Consumer: 1번째 이벤트

        alt 재고 있음 (1~10번)
            Consumer->>DB: 쿠폰 발급
            Consumer->>DB: request status = ISSUED
            Consumer->>DB: event_handled 저장
        else 재고 소진 (11~100번)
            Consumer->>DB: request status = FAILED
            Consumer->>DB: failureReason = "재고 소진"
            Note over Consumer: BusinessFailureException<br/>→ 재시도 안 함
        else 인프라 장애 (DB 타임아웃 등)
            Consumer->>Consumer: throw → Spring ErrorHandler
            Consumer->>Kafka: DLQ 즉시 전송
            Note over Consumer: 재시도 0회 (배치 블로킹 방지)
        end

        Consumer->>Kafka: ACK
    end

    Users->>API: GET /coupons/issue/{requestId}
    API->>DB: SELECT request
    API-->>Users: {status: ISSUED} or {status: FAILED}
Loading
Graceful Shutdown — 안전한 종료
sequenceDiagram
    autonumber
    participant K8s as Kubernetes
    participant App as Spring Boot App
    participant Relay as OutboxRelay
    participant DB as Database
    participant Kafka

    K8s->>App: SIGTERM (kill -15)

    activate App
    Note over App: Graceful Shutdown 시작

    App->>Relay: @PreDestroy 호출
    activate Relay

    Relay->>Relay: shuttingDown = true
    Note over Relay: 새 Phase 2 실행 중단

    alt Phase 2 실행 중
        Note over Relay: CountDownLatch 대기
        Relay->>Kafka: 진행 중인 발행 완료
        Kafka-->>Relay: ACK
        Relay->>DB: markPublished()
        Note over Relay: 최대 30초 대기
    end

    rect rgb(255, 240, 240)
        Note over Relay,DB: 복구: PROCESSING → PENDING
        Relay->>DB: SELECT status=PROCESSING
        Relay->>DB: UPDATE status=PENDING
        Note over Relay: 재시작 후 재발행 가능
    end

    Relay-->>App: Shutdown 완료
    deactivate Relay

    App-->>K8s: 종료 준비 완료
    deactivate App

    Note over K8s: 앱 종료 (데이터 유실 없음 ✅)
Loading
장애 흐름 — 예외 분류 및 복구
flowchart TD
    Start[Consumer 메시지 수신] --> Check{event_handled<br/>존재?}

    Check -->|Yes| Skip[스킵<br/>멱등성 보장 ✅]
    Check -->|No| Execute[비즈니스 로직 실행]

    Execute --> Result{결과?}

    Result -->|성공| Success[event_handled 저장<br/>+ ACK ✅]

    Result -->|Business<br/>Failure| BizFail[FAILED 기록<br/>재시도 안 함 ❌]
    BizFail --> ReasonLog[failureReason 저장<br/>예: 재고 소진]

    Result -->|인프라<br/>예외| Throw[throw → Spring ErrorHandler]

    Throw --> DLQ[즉시 DLQ 격리<br/>재시도 0회 — 배치 블로킹 방지]

    DLQ --> Manual[수동 확인 필요<br/>운영 알람 발송]

    style Success fill:#d4edda,stroke:#28a745
    style Skip fill:#d1ecf1,stroke:#17a2b8
    style BizFail fill:#f8d7da,stroke:#dc3545
    style DLQ fill:#fff3cd,stroke:#ffc107
    style Manual fill:#f8d7da,stroke:#dc3545
Loading

🧪 테스트

KafkaCouponConcurrencyTest — 선착순 동시성 (3건)

시나리오 동시 요청 쿠폰 수량 발급 거절 초과 발급
100명, 10장 100 (CountDownLatch) 10 10 90 0
200명, 50장 200 50 50 150 0
중복 멱등성 1명 2회 - 1 1 0

CouponIssueConsumerTest — Consumer 단위 (3건)

테스트 결과
발급 성공 → ISSUED + event_handled
재고 소진 → FAILED + failureReason
같은 eventId 2회 → 1건만 발급
OutboxMetricsTest — 모니터링 (7건) + OutboxGracefulShutdownTest — 안정성 (3건) + 기존 동시성 호환 (6건)

OutboxMetricsTest — 모니터링 (7건)

테스트 결과
발행 성공 카운터 증가
발행 실패 카운터 증가
Phase 1 처리 시간 기록
Phase 2 처리 시간 기록
PENDING 개수 Gauge 등록
PROCESSING 개수 Gauge 등록
모든 타이머 등록 확인

OutboxGracefulShutdownTest — 안정성 (3건)

테스트 결과
Graceful Shutdown 시 PROCESSING → PENDING 복원
5분 이상 PROCESSING 복구 로직 동작
Shutdown 플래그 설정 시 Phase 2 건너뜀

기존 동시성 테스트 호환 (5파일 6건)

테스트 결과
쿠폰 동시 사용 (원자적 UPDATE) — 10명/1장
쿠폰 동시 발급 (비관적 락) — 10명/5장
재고 동시 예약 (비관적 락) — 10명/5개
좋아요 동시 요청 — 10명
포인트 동시 차감 — 10명
주문 통합 동시성 — 10명/재고 5개

총 테스트: 29건 (모두 ✅)


⚖️ 트레이드오프

항목 현재 선택 검토한 대안 트레이드오프 판단 근거
Relay 지연 Polling 1초 (sync) CDC (Debezium), 하이브리드 (즉시 발행 + 폴링 보완) 이벤트 발행까지 최대 1~2초 CDC는 Debezium 클러스터 운영 비용이 Relay 구현 비용보다 큼. 하이브리드는 발행 경로 2개로 디버깅 복잡도 증가. Polling 처리량(실측 ~135건/초)이 충분하고 Outbox 테이블 유지 시 CDC 전환도 Relay 교체만으로 가능
Replica 1 단일 Broker RF=3, min.ISR=2 죽으면 유실 현재 가용성 요구 수준에서 복제 비용 불필요. kafka-pipeline-lab에서 RF=3, min.ISR=2 구성을 검증 완료하여 전환 경로 확보
CouponIssueConsumer 위치 commerce-api commerce-streamer에 배치 api에 Consumer 책임 추가 CouponService(도메인)가 commerce-api에 존재. streamer로 옮기면 도메인 로직 중복 또는 공유 모듈 추출 필요 — 현재 아키텍처에서는 api 배치가 변경 범위 최소
event_handled 보존 삭제 정책 미구현 Kafka retention 기반 TTL 배치 삭제 테이블 무한 증가 Consumer 멱등성 체크는 Kafka retention 이내 재전달만 방어하면 충분. retention(7일) 이상 보존 후 배치 삭제로 테이블 증가를 제어할 수 있으며, 구현 예정
product_metrics 분리 Product와 별도 테이블 products 테이블에 like_count/sales_count 컬럼 직접 추가 조회 시 JOIN Consumer가 고빈도 upsert하는 metrics와 관리자가 조회하는 Product가 같은 행을 잡으면 행 락 충돌. 별도 테이블로 쓰기/읽기 대상을 물리적으로 분리
방어적 락 유지 findByIdForUpdate() 존재 락 제거 (Kafka 파티션 순차 처리만 신뢰) 파티션 보장에 락 비용 추가 Kafka 외 경로에서 CouponService가 호출되는 경우 실재 — 현재 프로젝트의 KafkaCouponConcurrencyTest가 Consumer 없이 서비스를 직접 호출. 인프라 보장에만 의존하면 도메인 계층 방어가 사라짐
PROCESSING 복구 5분 threshold Phase 2가 5분 넘으면 복구 대상 heartbeat 방식 (Phase 2 중 주기적 updated_at 갱신), threshold 상향 (10분+) 중복 발행 가능성 실증: 건당 200ms 지연에서도 Phase 2는 12초(5분의 4%). 5분 도달하려면 Kafka 브로커 장기 무응답 필요. heartbeat는 DB 쓰기 추가로 Phase 2 처리량 감소. 장애 시에도 Consumer 멱등성이 중복 방어
event_handled 멱등성 비용 건당 ~10.9ms (SELECT + INSERT) Redis SET NX (인메모리 체크, DB 접근 제거) Consumer 처리량 ~50% 감소 Redis SET NX는 비즈니스 실패 시에도 "처리됨"이 남아 재시도 영구 불가. DB + 같은 TX이면 비즈니스 실패 시 event_handled도 롤백되어 재시도 가능. 실증: MISS 10.9ms, HIT 2.0ms(81.9% 절감). burst 시 Consumer Lag → 파티션 증설로 완화
parallelStream commonPool ForkJoinPool.commonPool() 사용 전용 ExecutorService (I/O 전용 스레드풀) I/O 블로킹에 CPU-bound 풀 사용 현재 사용처 1곳, commonPool의 다른 사용자 없음. 전용 풀은 스레드 수/큐 사이즈 튜닝 필요 — 단일 사용처에 과도. 프로덕션에서 commonPool 공유 사용처가 추가되면 전용 풀로 교체
Consumer 재시도 0회 Spring ErrorHandler → 즉시 DLQ Consumer 내부 재시도 (exponential backoff: 1초→2초→4초) 일시적 장애에도 DLQ 직행 배치 리스너에서 내부 backoff가 전체 배치를 블로킹 → 최악 500건 × 7초 = MAX_POLL_INTERVAL_MS(120초) 초과 → 무한 리밸런싱. DLQ 격리 후 수동 재처리가 안전
Graceful Shutdown 30초 대기 즉시 종료 + PROCESSING 복구 로직에 위임 배포 시간 증가 즉시 종료 시 Kafka 발행 성공했지만 markPublished 안 된 이벤트가 PROCESSING으로 잔류 → 복구 시 중복 발행. 30초 대기로 진행 중 발행을 완료시키면 중복 최소화. K8s terminationGracePeriodSeconds=30과 일치

🔍 리뷰 포인트

RP01: ApplicationEvent와 Outbox의 역할 분리 — 현업에서 이 두 도구를 통합하는 접근이 있는가

AFTER_COMMIT(TX 후 실행)과 Outbox 원자성(TX 안에서 저장)의 트랜잭션 타이밍 요구사항이 정반대라 통합에 실패했고, 결국 Facade에서 직접 호출하는 방식으로 분리했습니다.

🔗 LikeFacade.java#L60-L62 — Facade TX 안에서 Outbox 직접 저장

현재 경계: ApplicationEvent = 같은 JVM 내 부가 처리(캐시, 로깅), Outbox = 시스템 간 신뢰성 전달(Kafka).

궁금한 건 현업에서도 이렇게 분리하는 게 일반적인 접근인지입니다. 아니면 하나의 이벤트 인프라(예: 자체 EventBus)로 JVM 내 처리와 Kafka 발행을 모두 커버하는 설계가 더 흔한 건지, 그런 통합 설계에서는 TX 타이밍 문제를 어떻게 풀고 있는지 의견 부탁드립니다.


RP02: Kafka 파티션 순차 처리로 락을 대체했는데, 도메인 내 방어적 락을 유지해야 하는가

선착순 쿠폰을 key=couponTemplateId로 파티션 라우팅하여, 같은 쿠폰에 대한 요청이 같은 파티션으로 라우팅되어 단일 Consumer가 순차 처리하도록 구현했습니다. (murmur2(key) % partitionCount 파티션 수가 고정인 한 동일 파티션 보장. 파티션 확장 시 라우팅이 변경되므로 사전 재배치 계획 필요.) 동시 접근 자체가 불가능한 구조이고, 실제로 100명/200명 동시 요청에서 초과 발급 0건을 확인했습니다:

🔗 KafkaCouponConcurrencyTest.java#L87-L150 — 100명/10장 + 200명/50장 동시성 테스트

🔗 CouponIssueConsumer.java#L45-L64 — Consumer 예외 분류 (BusinessFailureException catch + 인프라 예외 throw)
🔗 CouponIssueProcessor.java@transactional 비즈니스 로직

그런데 CouponService 내부에 findByIdForUpdate()가 여전히 남아있습니다. 파티션이 순서를 보장하고 있는데 이 락이 필요한가?라는 고민이 있습니다.

유지하는 근거는 "Kafka가 아닌 경로에서 CouponService가 호출될 가능성"입니다. 관리자 API에서 직접 호출하거나, 테스트에서 Consumer 없이 서비스를 호출하는 경우가 있을 수 있습니다. 지금 이 프로젝트에서도 KafkaCouponConcurrencyTest가 Consumer를 직접 호출하는 방식으로 테스트하고 있어서, 파티션 보장 바깥에서 서비스가 호출되는 상황이 실제로 존재합니다.

반대로 제거 근거는, 파티션 보장이 있는 상태에서 락은 SELECT FOR UPDATE + 대기 시간이라는 성능 비용만 추가한다는 점입니다. 특히 순차 처리가 보장된 상황에서 행 잠금은 매번 즉시 획득되므로 실질적인 동시성 보호 효과 없이 오버헤드만 남습니다.

"인프라 보장(Kafka 파티션)을 믿지 말고 도메인에서 방어하라"는 원칙과 "불필요한 락은 제거하라"는 원칙이 충돌하는데, 현업에서는 이 상황을 어떤 기준으로 판단하시나요? 특히 Kafka 기반으로 동시성을 제어하는 구조에서, 도메인 서비스의 기존 락을 그대로 유지하는 접근과 Kafka 도입 시 함께 정리하는 접근 중 어느 것이 더 적절한지 궁금합니다.


RP03: 선착순에서 Outbox Polling 지연을 수용할 수 있는가

부하 테스트 결과, 5000건 burst를 소화하는 데 32.3초(~155건/초)가 걸렸습니다. 병목은 Phase 2의 동기 .get(10s) Kafka I/O입니다. 1만 명이 동시 요청하면 실측 기준 약 65초 — 10,000번 유저는 1분 이상을 기다립니다.

선착순이라는 도메인 특성이 고민입니다. 선착순은 "빠른 응답"이 UX의 핵심이고, "선착순 쿠폰 발급 중입니다 (20초 대기)"는 유저 수용도가 매우 낮습니다. Polling인 이상 "읽고 → 발행하고 → 다음 주기 대기"라는 구조적 한계가 있습니다.

하이브리드(AFTER_COMMIT 즉시 발행 + Outbox fallback)도 검토했지만, 발행 경로가 2개로 늘어나는 디버깅 복잡도와 멱등성 부담 때문에 구현하지 않았습니다.

궁금한 건 실무에서 이런 burst 시나리오를 만났을 때, Relay 튜닝으로 가는지, 아니면 선착순처럼 즉시성이 필요한 도메인에서는 Outbox를 우회하고 직접 Kafka 발행(+ 실패 시 보정)도 현실적인 선택지인지입니다. 쿠폰 발급 요청이라는 특성상 "요청 유실 → 재요청"이 가능하다면 Dual Write 위험을 수용할 수 있는 건지, 이런 판단의 기준이 궁금합니다.


📁 변경 파일

주요 신규 파일

파일 역할
OutboxEventEntity.java Outbox 테이블 (PENDING → PROCESSING → PUBLISHED/FAILED) + updated_at
OutboxEventService.java Facade TX 안에서 Outbox 저장
OutboxRelayService.java 2-Phase Relay + Graceful Shutdown + 복구 로직
OutboxStatus.java PENDING / PROCESSING / PUBLISHED / FAILED 상태 정의
OutboxEventJpaRepository.java FOR UPDATE SKIP LOCKED 네이티브 쿼리
OutboxMetrics.java Prometheus 메트릭 9개 (Gauge/Timer/Counter)
EventHandledEntity.java Consumer 멱등성 (UNIQUE event_id)
CatalogMetricsConsumer.java 상품 메트릭 집계 Consumer
CatalogMetricsProcessor.java @transactional 메트릭 처리 (Consumer와 분리 — self-invocation 해결)
CouponIssueConsumer.java 선착순 쿠폰 Consumer (예외 분류 + Spring ErrorHandler 위임)
CouponIssueProcessor.java @transactional 쿠폰 발급 (Consumer와 분리 — self-invocation 해결)
DlqPublisher.java (commerce-api) DLQ 동기 격리 + X-Retry-Count
DlqPublisher.java (commerce-streamer) DLQ 동기 격리
KafkaTopicConfig.java 5개 토픽 정의
ProductMetricsEntity.java 상품 메트릭 테이블
이벤트 클래스 (ProductLikedEvent, OrderItemSoldEvent, CouponIssueRequestedEvent 등) 이벤트 record + Outbox 페이로드
IdempotencyService, EventHandledJpaRepository 등 멱등성 인프라
테스트 8개 (29건) 동시성, Consumer, Outbox 통합/성능, 메트릭, Graceful Shutdown

수정 (5개)

파일 변경
LikeFacade.java Outbox 이벤트 발행 추가
OrderFacade.java Outbox 이벤트 발행 추가
CouponFacade.java 비동기 쿠폰 발급 (Outbox + 202 Accepted)
kafka.yml Producer (acks=all, idempotence=true) + Consumer (manual ACK)
build.gradle.kts kafka 모듈 의존성

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

아웃박스 기반 Kafka 발행·릴레이와 DLQ/컨슈머 멱등성 인프라, 쿠폰 비동기 발급(요청/조회) 및 애플리케이션 계층에서의 아웃박스 저장 호출을 도입하고 관련 통합·동시성 테스트를 추가했다.

Changes

Cohort / File(s) Summary
빌드·설정
apps/commerce-api/build.gradle.kts, apps/commerce-api/src/main/resources/application.yml, modules/kafka/src/main/resources/kafka.yml
modules:kafka 의존성 추가 및 kafka.yml 임포트; Kafka 프로듀서에 acks: all, idempotence 및 in-flight 설정 추가.
토픽 설정
apps/commerce-api/src/main/java/com/loopers/config/TopicProperties.java, apps/commerce-api/src/main/java/com/loopers/config/KafkaTopicConfig.java, apps/commerce-api/src/main/resources/application-local.yml, apps/commerce-api/src/main/resources/application-prd.yml
TopicProperties와 KafkaTopicConfig 추가로 5개 토픽(catalog-events-v1, order-events-v1, coupon-issue-requests-v1, user-activity-events-v1, pipeline-dlq-v1)을 NewTopic 빈으로 등록함.
비동기·스케줄 설정
apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java
@EnableAsync@EnableScheduling 활성화로 비동기 리스너 및 스케줄러 사용 가능하게 함.
아웃박스 도메인·저장·서비스
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/...
OutboxEventEntity/OutboxStatus/OutboxEventJpaRepository/OutboxEventService 추가 및 엔티티 상태 전이 API 제공.
아웃박스 릴레이·메트릭·스케줄
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java, apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java, db/migration/V1__Add_updated_at_and_indexes.sql
주기적 PENDING→PROCESSING 선정, Kafka 동기 전송, PUBLISHED/FAILED 전이, 재시도·복구·정리 스케줄, Micrometer 계측 및 DB 인덱스 마이그레이션 추가.
멱등성 추적 엔티티·서비스
apps/commerce-api/src/main/java/.../event/*, apps/commerce-streamer/src/main/java/.../event/*
EventHandledEntity 및 EventHandledJpaRepository/IdempotencyService 추가로 소비자측 중복 처리·검증 지원.
쿠폰 비동기 요청 도메인·리포지토리
apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/*, apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponFacade.java
CouponIssueRequestEntity, CouponIssueRequestStatus, JPA 리포지토리 추가; CouponFacade에 requestCouponIssue/getCouponIssueResult 및 outbox 저장 연계 추가.
쿠폰 이슈 컨슈머·DLQ
apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java, apps/commerce-api/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java, apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java
쿠폰 이슈 배치 컨슈머 추가(멱등성 검사, 트랜잭션 처리, 예외 분기: BusinessFailure는 로깅만, 인프라 예외는 컨테이너가 DLQ 담당); DLQ 동기 퍼블리셔 도입.
애플리케이션 레이어: outbox 호출 및 이벤트 퍼블리시
apps/commerce-api/src/main/java/com/loopers/application/{like,order,payment,product}/*
LikeFacade/OrderFacade/PaymentFacade에 OutboxEventService 주입 및 트랜잭션 내 outbox 저장 추가; ProductFacade는 ApplicationEventPublisher 주입해 ProductViewedEvent 발행; LikeFacade는 ProductLikedEvent 저장.
스트리머 소비자: 메트릭 집계
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java
catalog-events 배치 소비자 추가(헤더 파싱, payload 기반 ProductMetrics 업데이트, 멱등성 확인, 실패 시 DLQ 전송).
제품 메트릭 엔티티
apps/commerce-api/src/main/java/.../product/*, apps/commerce-streamer/src/main/java/.../product/*
ProductMetricsEntity 및 ProductMetricsJpaRepository 추가, 뷰/좋아요/판매 카운터 변경 API 제공.
이벤트 리스너
apps/commerce-api/src/main/java/com/loopers/application/product/ProductEventListener.java
ProductViewedEvent를 비동기로 수신해 로깅 처리하는 리스너 추가.
테스트
apps/commerce-api/src/test/java/..., apps/commerce-streamer/src/test/java/...
Outbox·릴레이·DLQ·컨슈머·쿠폰 비동시성 관련 통합/단위 테스트 대거 추가(OutboxEventServiceTest, OutboxRelayServiceIntegrationTest, CouponIssueConsumerTest, KafkaCouponConcurrencyTest 등).
제거
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/DemoKafkaConsumer.java
레거시 데모 Kafka 컨슈머 제거.

Sequence Diagram(s)

sequenceDiagram
    participant App as 애플리케이션(Facade)
    participant DB as DB(outbox_event)
    participant Relay as OutboxRelayService
    participant Kafka as Kafka Broker

    App->>DB: 트랜잭션 내 OutboxEventEntity 생성(status=PENDING)
    Note over App,DB: 비즈니스 변경과 outbox 저장이 같은 TX에서 수행된다.
    Relay->>DB: 주기적으로 PENDING 이벤트 조회(for update / skip locked)
    DB-->>Relay: 이벤트 목록 반환
    Relay->>Kafka: 각 이벤트 동기 전송(.send().get(10s))
    alt 전송 성공
        Relay->>DB: 이벤트 상태를 PUBLISHED로 업데이트(publishedAt 기록)
    else 전송 실패
        Relay->>DB: retryCount 증가 및 상태를 FAILED로 업데이트
    end
    Relay->>DB: 오래된 PUBLISHED 이벤트 삭제(청소 스케줄)
Loading
sequenceDiagram
    participant Kafka as Kafka Broker
    participant Consumer as Batch Consumer
    participant Idem as IdempotencyService
    participant AppDB as DB(도메인 / event_handled)
    participant DLQ as DlqPublisher

    Kafka->>Consumer: 배치 수신(헤더 포함)
    loop 각 레코드
        Consumer->>Idem: outboxId 존재 여부 확인
        Idem->>AppDB: existsByEventId 쿼리
        AppDB-->>Idem: 결과 반환
        alt 이미 처리됨
            Consumer->>Consumer: 레코드 스킵
        else 미처리
            Consumer->>AppDB: 트랜잭션 내 도메인 처리(ProductMetrics 등)
            alt 처리 성공
                Consumer->>AppDB: EventHandledEntity 저장(멱등성 기록)
            else 처리 실패
                Consumer->>DLQ: 실패 레코드 동기 전송(에러 정보 포함)
            end
        end
    end
    Consumer->>Kafka: 배치 오프셋 커밋(ACK)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive PR 설명에서 GitHub 이슈 링크가 명시적으로 표시되지 않아 관련 이슈와의 연결 여부를 확인할 수 없다. GitHub 이슈와의 연결 상태를 확인하거나, 해당하는 경우 설명에 이슈 번호를 추가하길 권장한다.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed PR 설명은 배경·목표·결과·설계 의사결정·아키텍처·플로우·테스트·트레이드오프·리뷰 포인트를 체계적으로 기록하고 있다.
Title check ✅ Passed PR 제목이 명확하게 주요 변경사항(ApplicationEvent 경계 분리, Outbox Pipeline, 선착순 쿠폰)을 나타낸다.
Out of Scope Changes check ✅ Passed 변경사항이 명확한 목표(이벤트 기반 아키텍처 개선)에 집중하고 있으며, 범위 내 변경만 포함하고 있다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
modules/kafka/src/main/resources/kafka.yml (1)

24-25: ⚠️ Potential issue | 🔴 Critical

consumer value-serializer 오타로 역직렬화 실패 발생

Line 25에서 value-serializer로 되어 있으나, consumer 설정이므로 value-deserializer가 올바른 키이다. 이 오타로 인해 consumer가 기본 deserializer를 사용하게 되어 메시지 역직렬화 실패 또는 예상치 못한 동작이 발생할 수 있다.

운영 환경에서 Kafka 메시지 소비 장애로 이어질 수 있는 critical 이슈이다.

🐛 오타 수정
     consumer:
       group-id: loopers-default-consumer
       key-deserializer: org.apache.kafka.common.serialization.StringDeserializer
-      value-serializer: org.apache.kafka.common.serialization.ByteArrayDeserializer
+      value-deserializer: org.apache.kafka.common.serialization.ByteArrayDeserializer
       properties:
         enable-auto-commit: false

수정 후 consumer 통합 테스트로 메시지 소비 정상 동작을 검증해야 한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/kafka/src/main/resources/kafka.yml` around lines 24 - 25, kafka
consumer 설정에서 키 `value-serializer`가 오타로 사용되어 역직렬화가 실패하니 `value-serializer`를
`value-deserializer`로 변경하고(현재 설정의 다른 키인 `key-deserializer`는 유지), 변경한
항목(`value-deserializer:
org.apache.kafka.common.serialization.ByteArrayDeserializer`)으로 consumer 통합 테스트를
실행해 메시지 소비가 정상 동작하는지 검증하세요.
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (1)

55-70: ⚠️ Potential issue | 🟠 Major

캐시 히트 경로에서 조회 이벤트가 누락된다

Line [57]~Line [59]에서 즉시 반환되어 Line [67]~Line [69]가 실행되지 않으므로, 운영 지표 기준 조회 수가 실제 트래픽보다 지속적으로 낮아진다. 캐시 효율이 높아질수록 왜곡이 커져 모니터링/의사결정 품질이 떨어진다.

  • 문제 이유(운영 관점): 조회 집계와 사용자 행동 로그가 캐시 미스에만 기록되어 KPI가 과소계상된다.
  • 수정안: 캐시 히트/미스와 무관하게 이벤트를 발행하도록 공통 경로로 이동한다.
  • 추가 테스트:
    1. 캐시 히트 시에도 ProductViewedEvent가 1회 발행되는지 검증한다.
    2. 캐시 미스 시에도 기존과 동일하게 1회 발행되는지 검증한다.
    3. 동일 productId 연속 호출에서 히트/미스 조합별 발행 횟수 합이 호출 횟수와 일치하는지 검증한다.
수정 예시
 public ProductDetailResult getProductDetail(Long productId) {
     Optional<ProductDetailResult> cached = productCacheManager.getProductDetail(productId);
     if (cached.isPresent()) {
-        return cached.get();
+        eventPublisher.publishEvent(new ProductViewedEvent(null, productId));
+        return cached.get();
     }

     Product product = productService.getDisplayableProduct(productId);
     Brand brand = brandService.getActiveBrand(product.getBrandId());
     ProductDetailResult result = new ProductDetailResult(ProductInfo.from(product), BrandInfo.from(brand));

     productCacheManager.putProductDetail(productId, result);
-
-    // 상품 조회 이벤트 발행 — product_metrics 조회 수 집계 + 유저 행동 로깅 (추후 Kafka 전환)
-    // userId는 현재 컨텍스트에서 가져올 수 없으므로 null 허용 (비로그인 조회)
     eventPublisher.publishEvent(new ProductViewedEvent(null, productId));

     return result;
 }
🤖 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 55 - 70, The cache-hit path currently returns early from
ProductFacade.getProductDetail after productCacheManager.getProductDetail,
skipping eventPublisher.publishEvent; move the ProductViewedEvent publishing
into a single common exit path so publishEvent is called for both cache hits and
misses (i.e., ensure productCacheManager.getProductDetail -> assign to result
variable, if missing populate result via productService/brandService and
productCacheManager.putProductDetail, then call eventPublisher.publishEvent(new
ProductViewedEvent(null, productId)) before returning result). Update or add
unit tests to assert ProductViewedEvent is published exactly once for cache-hit,
cache-miss, and mixed consecutive calls.
🟠 Major comments (16)
apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestEntity.java-49-50 (1)

49-50: ⚠️ Potential issue | 🟠 Major

실패 사유에 raw 예외 메시지를 저장하면 내부 정보가 노출될 수 있다

Line 76~79에서 예외 원문을 그대로 failureReason에 저장하면 사용자 조회 응답으로 내부 구현/DB 정보가 유출될 수 있고, 길이 초과 시 업데이트 실패로 재처리 루프가 생길 수 있다. 사용자용 실패 코드는 별도 필드(예: failureCode)로 저장하고, 저장 메시지는 정규화/길이 제한을 적용하며, 긴 메시지 및 민감 키워드 포함 메시지에 대한 저장/노출 테스트를 추가해야 한다.

As per coding guidelines **/*.java: "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."

Also applies to: 76-79

🤖 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/coupon/CouponIssueRequestEntity.java`
around lines 49 - 50, The field failureReason on CouponIssueRequestEntity
currently stores raw exception texts; change the persistence model to add a
separate failureCode field for user-facing codes and stop saving full exception
causes into failureReason, instead store a normalized, length-limited (<=200)
and sanitized summary (truncate with ellipsis, strip sensitive keywords like
passwords/SQL/PII) in failureReason; preserve the original exception cause only
in application logs (or a non-user-facing audit column if needed) and update the
code that writes to failureReason (look for assignments around failureReason and
the exception handling block that references lines 76-79) to perform
normalization/truncation and set failureCode appropriately, plus add
unit/integration tests verifying long messages, messages with sensitive
keywords, and that failureCode vs failureReason separation is enforced.
apps/commerce-api/src/main/java/com/loopers/domain/common/event/OrderItemSoldEvent.java-13-16 (1)

13-16: ⚠️ Potential issue | 🟠 Major

가변 Map 참조가 이벤트 불변성을 깨뜨린다

Line 15에서 외부 Map 참조를 그대로 보관하면 이벤트 생성 후 데이터가 바뀔 수 있어, 운영 중 집계값 왜곡과 재현 어려운 장애로 이어진다. compact constructor에서 Map.copyOf와 항목 검증(키/수량 null 금지, 수량 양수)을 적용하고, 원본 Map 변경이 이벤트에 반영되지 않는지 테스트를 추가해야 한다.

수정안 예시
+import java.util.Map;
+import java.util.Objects;
+
 public record OrderItemSoldEvent(
         Long orderId,
         Map<Long, Integer> productQtyMap
 ) {
+    public OrderItemSoldEvent {
+        Objects.requireNonNull(orderId, "orderId must not be null");
+        Objects.requireNonNull(productQtyMap, "productQtyMap must not be null");
+        productQtyMap = Map.copyOf(productQtyMap);
+        productQtyMap.forEach((productId, qty) -> {
+            if (productId == null || qty == null || qty <= 0) {
+                throw new IllegalArgumentException("productQtyMap contains invalid entry");
+            }
+        });
+    }
 }

As per coding guidelines **/domain/**/*.java: "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/common/event/OrderItemSoldEvent.java`
around lines 13 - 16, OrderItemSoldEvent currently stores the passed
productQtyMap reference which breaks event immutability; add a compact
constructor in the OrderItemSoldEvent record that copies the map with
Map.copyOf(productQtyMap) and validates entries (no null keys/values, quantities
> 0), throwing IllegalArgumentException for invalid data, and update or add a
unit test to confirm that mutating the original map after construction does not
affect the event's internal map.
apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.java-11-12 (1)

11-12: ⚠️ Potential issue | 🟠 Major

비유니크 조건을 Optional로 조회하면 런타임 예외 위험이 있다

Line 11~12는 유니크 제약이 없는 조건인데 Optional 단건 조회를 사용해, 동일 상태 요청이 2건 이상이면 운영 중 IncorrectResultSize 계열 예외가 발생할 수 있다. 최신 1건을 명시적으로 조회(findTopBy...OrderByRequestedAtDesc)하거나 다건 반환으로 바꾸고, 동일 (userId, templateId, status) 데이터 2건 존재 시 동작을 검증하는 테스트를 추가해야 한다.

수정안 예시
-Optional<CouponIssueRequestEntity> findByUserIdAndCouponTemplateIdAndStatus(
-        Long userId, Long couponTemplateId, CouponIssueRequestStatus status);
+Optional<CouponIssueRequestEntity> findTopByUserIdAndCouponTemplateIdAndStatusOrderByRequestedAtDesc(
+        Long userId, Long couponTemplateId, CouponIssueRequestStatus status);

Based on learnings "the issue() method ... does NOT implement concurrent duplicate issuance prevention ... deferred to a later phase."

🤖 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/coupon/CouponIssueRequestJpaRepository.java`
around lines 11 - 12, The repository method
findByUserIdAndCouponTemplateIdAndStatus in CouponIssueRequestJpaRepository uses
Optional for a non-unique query and can throw IncorrectResultSize at runtime if
multiple rows exist; change it to either return the latest single row with a
query like findTopByUserIdAndCouponTemplateIdAndStatusOrderByRequestedAtDesc or
change the return type to List<CouponIssueRequestEntity> to explicitly handle
multiple results, then update callers (including issue()) to handle the new
return shape; also add a unit/integration test that inserts two records with the
same (userId, couponTemplateId, status) and verifies the chosen behavior (latest
selected or multiple returned) to prevent regressions.
apps/commerce-api/src/main/java/com/loopers/domain/common/event/UserActivityEvent.java-14-35 (1)

14-35: ⚠️ Potential issue | 🟠 Major

이벤트 생성 시 필수값 검증이 없어 무효 페이로드가 유입될 수 있다

Line 14~20에서 userId/targetId/occurredAt가 null이어도 객체가 생성되어, 운영 시 소비자 예외 및 DLQ 적재 증가로 이어질 수 있다. compact constructor에서 필수값 검증과 ID 양수 검증을 추가하고, 무효 입력 시 예외 발생 테스트와 팩토리 메서드 정상 생성 테스트를 보강해야 한다.

수정안 예시
+import java.util.Objects;
+
 public record UserActivityEvent(
         Long userId,
         String activityType,
         String targetType,
         Long targetId,
         LocalDateTime occurredAt
 ) {
+    public UserActivityEvent {
+        Objects.requireNonNull(userId, "userId must not be null");
+        Objects.requireNonNull(activityType, "activityType must not be null");
+        Objects.requireNonNull(targetType, "targetType must not be null");
+        Objects.requireNonNull(targetId, "targetId must not be null");
+        Objects.requireNonNull(occurredAt, "occurredAt must not be null");
+        if (userId <= 0 || targetId <= 0) {
+            throw new IllegalArgumentException("ids must be positive");
+        }
+    }

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/domain/common/event/UserActivityEvent.java`
around lines 14 - 35, Add null and ID-positivity validation in the
UserActivityEvent compact constructor so invalid events cannot be created; check
that userId and targetId are non-null and >0 and occurredAt is non-null, and
throw an appropriate runtime exception (e.g.,
IllegalArgumentException/NullPointerException) with a clear message when
validation fails. Update the factory methods view/like/order/payment remain
unchanged but rely on the constructor validation; add unit tests that assert
exceptions are thrown for null/negative userId/targetId/occurredAt and tests
that validate factory methods create valid instances.
apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsEntity.java-43-73 (1)

43-73: ⚠️ Potential issue | 🟠 Major

메트릭 누적 메서드의 입력 검증이 없어 집계값이 오염될 수 있다

Line 70~73에서 quantity <= 0 입력을 허용하면 판매량이 감소하거나 무의미한 업데이트가 발생해 운영 지표 신뢰도가 떨어진다. createaddSalesCount에 선제 검증을 넣고, 음수/0 수량 거부 테스트와 정상 누적 테스트를 추가해야 한다.

수정안 예시
+import java.util.Objects;
+
 public static ProductMetricsEntity create(Long productId) {
+    Objects.requireNonNull(productId, "productId must not be null");
     ProductMetricsEntity entity = new ProductMetricsEntity();
@@
 public void addSalesCount(int quantity) {
+    if (quantity <= 0) {
+        throw new IllegalArgumentException("quantity must be positive");
+    }
     this.salesCount += quantity;
     this.updatedAt = ZonedDateTime.now();
 }

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/infrastructure/product/ProductMetricsEntity.java`
around lines 43 - 73, Validate inputs in ProductMetricsEntity: in the static
factory method create(Long productId) check productId is not null and throw
IllegalArgumentException if null, and in addSalesCount(int quantity) reject
non-positive values by throwing IllegalArgumentException (do not allow quantity
<= 0) and only update salesCount and updatedAt when valid; update unit tests to
cover create(null) rejection, addSalesCount(0)/addSalesCount(-1) rejection, and
successful addSalesCount positive accumulation to prevent metric contamination.
apps/commerce-api/src/main/java/com/loopers/domain/common/event/ProductLikedEvent.java-16-21 (1)

16-21: ⚠️ Potential issue | 🟠 Major

이벤트 설계 일관성 및 필드 검증 개선 필요

같은 ProductLikedEvent 레코드를 좋아요와 취소 두 경우에 모두 사용하면서 X-Event-Type 헤더로 "ProductLikedEvent"와 "ProductUnlikedEvent"를 구분하고 있다. LikeFacade에서 확인한 바:

  • 좋아요: new ProductLikedEvent(userId, productId, true) → "ProductLikedEvent" 헤더
  • 취소: new ProductLikedEvent(userId, productId, false) → "ProductUnlikedEvent" 헤더

문제점:

  1. 의미 중복: boolean liked 필드와 X-Event-Type 헤더가 동일한 정보를 중복 전달한다. CatalogMetricsConsumer는 헤더만 사용하고 liked 필드는 전혀 사용하지 않는다.
  2. 일관성 위험: 발행자 버그로 liked 값이 헤더와 맞지 않으면, 소비자가 잘못된 작업(증가/감소)을 수행할 수 있다.
  3. 도메인 모델 명확성: 레코드 이름이 "ProductLikedEvent"인데 취소 이벤트도 표현하므로 의미가 모호하다.
  4. null 검증 부재: userId, productId에 대한 방어적 검증이 없다.

개선안:

  • ProductLikedEventProductUnlikedEvent를 별도의 레코드로 분리하거나, liked 필드를 제거하고 이벤트 타입만 의존한다.
  • 레코드 컴팩트 생성자에서 userId != null && productId != 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/domain/common/event/ProductLikedEvent.java`
around lines 16 - 21, Split the ambiguous ProductLikedEvent into two explicit
event records (e.g., ProductLikedEvent and ProductUnlikedEvent) or remove the
boolean liked field so the X-Event-Type header is the single source of truth;
ensure the publisher (LikeFacade) emits the matching record type/header to avoid
header/payload divergence referenced by CatalogMetricsConsumer, and add a
compact constructor in each record to defensively validate userId and productId
are non-null (throw IllegalArgumentException) so null IDs cannot be constructed;
also add tests that assert behavior when header and payload disagree to catch
publisher bugs.
apps/commerce-api/src/main/java/com/loopers/config/KafkaTopicConfig.java-23-79 (1)

23-79: ⚠️ Potential issue | 🟠 Major

토픽 복제 계수 replicas(1) 하드코딩은 운영 환경에서 심각한 신뢰성 리스크를 야기한다

라인 27, 39, 51, 64, 76에서 모든 토픽의 복제 계수가 1로 고정되어 있으며, KafkaTopicConfig 클래스에 @ConditionalOnProfile이 없어 local, dev, qa, prd 모든 환경에서 동일하게 적용된다. 첫 토픽의 주석 "로컬 단일 Broker"는 실제로는 local 프로파일이 기본값일 뿐이며, 프로파일 전환 시에도 RF=1이 고정되는 문제가 발생한다.

  • 운영 리스크: 브로커 장애/점검 시 해당 파티션의 가용성 손실 및 메시지 손실 가능성 증대. 신뢰성 강화를 목표로 한 PR(메시지 유실 방어)의 취지와 상충한다.
  • 수정안: 복제 계수를 @Value로 외부화하고, application-{profile}.yml에서 환경별 기본값을 정의한다. 운영 환경은 최소 RF ≥ 3으로 설정하고, 클러스터 정책에 맞춰 조정한다.
  • 추가 테스트:
    1. Spring 프로파일 통합 테스트로 dev/qa/prd 환경에서 토픽 설정값이 올바르게 주입되는지 검증한다.
    2. 운영 설정 검증 테스트에서 필수 토픽의 복제 계수가 정책값(≥ 2) 이상인지 확인한다.
🤖 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/config/KafkaTopicConfig.java`
around lines 23 - 79, KafkaTopicConfig currently hardcodes replicas(1) in
methods catalogEventsTopic, orderEventsTopic, couponIssueRequestsTopic,
userActivityEventsTopic and dlqTopic; change this to inject a configurable
replica count (e.g., `@Value`("${kafka.topic.replication-factor}") or a per-topic
property) and use that value for TopicBuilder.replicas(...), add defaults in
application-{profile}.yml with production set to >=3, and add a startup or
integration test that asserts the injected replication factor meets your minimum
policy (e.g., >=2/3) for required topics to prevent RF=1 in non-local profiles.
apps/commerce-streamer/src/main/java/com/loopers/infrastructure/event/IdempotencyService.java-30-38 (1)

30-38: ⚠️ Potential issue | 🟠 Major

DataIntegrityViolationException 전체를 중복으로 처리하면 데이터 오류를 은닉할 수 있다

markHandled() 메서드(35~37줄)는 모든 무결성 예외를 중복 이벤트로 간주한다. EventHandledEntity의 제약은 eventId UNIQUE 외에도 topic NOT NULL, eventId/topic 길이 제약이 있다. 따라서 topic이 null이거나 길이가 100자를 초과하면 DataIntegrityViolationException이 발생하지만, 로그에는 "[Idempotency] 중복 감지 (UNIQUE 위반)"로 기록되어 실제 데이터 결함을 중복으로 오인하게 된다.

현재 EventHandledEntity.of()는 매개변수 검증을 수행하지 않으므로, 호출자가 사전 검증을 하지 않으면 이 문제가 발생한다. 더 정확하게는 eventId와 topic이 유효한 범위 내에 있는지 확인해야 한다.

  • 운영 영향: 실제 데이터 오류(NOT NULL 위반, 길이 초과)가 "중복"으로 기록되어 DB 정합성 문제를 놓치고 누적 장애로 이어질 수 있다.
  • 수정안: markHandled() 또는 EventHandledEntity.of()에서 eventId와 topic의 null 체크 및 길이 검증을 추가한 후, DataIntegrityViolationException을 제약별로 분기한다. UNIQUE 위반(eventId 중복)일 때만 false를 반환하고, 그 외 위반은 예외를 재던지도록 한다.
  • 추가 테스트:
    1. 동일 eventId 재삽입 시 false 반환을 검증한다.
    2. 다른 무결성 위반(topic null, 길이 초과) 시 예외가 전파되는지 검증한다.
    3. 로그에 중복 케이스와 비중복 실패 케이스가 구분되어 기록되는지 검증한다.

기술 가이드라인 **/*Service*.java: 멱등성과 중복 처리 방지 전략을 점검한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/event/IdempotencyService.java`
around lines 30 - 38, markHandled() treats any DataIntegrityViolationException
as a duplicate; validate inputs and narrow handling so real data errors aren't
masked: add null and length checks for eventId/topic either in
EventHandledEntity.of(...) or at start of markHandled(String eventId, String
topic) (validate not null and within allowed length), and when catching
DataIntegrityViolationException inspect the underlying cause (e.g.,
ConstraintViolationException / SQLState / SQLException error code or constraint
name) and only return false for the UNIQUE constraint on eventId; rethrow the
exception for NOT NULL/length/other constraint violations. Also add tests to
assert duplicate insert returns false, NOT NULL/length violations propagate, and
logs distinguish duplicate vs other failures.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventService.java-37-39 (1)

37-39: ⚠️ Potential issue | 🟠 Major

직렬화 실패를 일반 RuntimeException으로 던지면 에러 처리 일관성이 깨진다

Line 37-39는 원인 보존은 되지만 예외 타입이 표준화되지 않아 API 에러 포맷 일관성이 흔들릴 수 있다. CoreException 계열로 변환하고 사용자 메시지와 내부 로그 메시지를 분리하는 방식으로 수정하는 것이 필요하다. 수정 후에는 직렬화 실패 payload를 사용해 ApiControllerAdvice가 기대한 에러 스키마로 응답하는지 테스트를 추가해야 한다.
Based on learnings "enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format."

🤖 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/outbox/OutboxEventService.java`
around lines 37 - 39, Replace the generic RuntimeException thrown in
OutboxEventService's JsonProcessingException catch with a CoreException subtype
so errors flow through ApiControllerAdvice; in the catch block for
JsonProcessingException e inside OutboxEventService, create and throw a
CoreException (or an appropriate existing CoreException subclass) that preserves
the cause (e) but accepts a user-facing message (e.g., "페이로드 직렬화 실패") and
separate internal log details; update logging to log the full exception
internally before throwing the CoreException, and add a unit/integration test
that triggers the serialization failure to assert the API response matches the
error schema produced by ApiControllerAdvice.
apps/commerce-streamer/src/main/java/com/loopers/infrastructure/product/ProductMetricsEntity.java-61-63 (1)

61-63: ⚠️ Potential issue | 🟠 Major

음수 수량이 허용되어 판매 집계가 오염될 수 있다

Line 61-63에서 quantity가 음수여도 누적되므로, 잘못된 이벤트 입력 시 salesCount가 감소하여 운영 지표 정합성이 깨진다. quantity <= 0을 즉시 차단하도록 검증을 추가하는 것이 필요하다. 수정 후에는 음수/0 입력 시 예외가 발생하고 salesCount가 변하지 않는 테스트를 추가해야 한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/product/ProductMetricsEntity.java`
around lines 61 - 63, The addSalesCount method currently allows non-positive
quantities which can corrupt salesCount; add a guard in
ProductMetricsEntity.addSalesCount(int quantity) to validate quantity > 0 and if
not, throw an IllegalArgumentException (or a domain-specific exception) before
mutating salesCount or updatedAt (avoid calling ZonedDateTime.now() on invalid
input). Update logic so only when quantity > 0 you increment this.salesCount and
set this.updatedAt = ZonedDateTime.now(); and add unit tests that call
addSalesCount with 0 and a negative value asserting the exception is thrown and
that salesCount and updatedAt remain unchanged.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java-18-19 (1)

18-19: ⚠️ Potential issue | 🟠 Major

cleanupPublishedEvents에서 상한 없이 조회하면 대량 데이터 환경에서 메모리/락 점유가 급증한다

findPublishedBefore는 다른 배치 메서드(findPendingEvents, findRetryableEvents)와 달리 LIMIT 절이 없어서 7일 이상 누적된 PUBLISHED 이벤트를 모두 메모리에 로드한다. cleanupPublishedEvents는 1시간 주기로 실행되는데, 고부하 환경에서 수만 건의 레코드를 한 번에 조회하고 deleteAll 처리하면서 트랜잭션이 장시간 락을 점유하게 된다.

findPublishedBefore에 limit 파라미터를 추가하고, cleanupPublishedEvents에서 청크 단위로 반복 조회·삭제하도록 수정한다. BATCH_SIZE(50)를 활용하여 다른 배치 메서드와 일관성 있게 처리한다.

`@Query`("SELECT o FROM OutboxEventEntity o WHERE o.status = 'PUBLISHED' AND o.publishedAt < :before ORDER BY o.id ASC LIMIT :limit")
List<OutboxEventEntity> findPublishedBefore(`@Param`("before") ZonedDateTime before, `@Param`("limit") int limit);

cleanupPublishedEvents에서는 비어질 때까지 limit 크기만큼 반복 조회·삭제하도록 한다. 수정 후 대량 데이터 조건에서 1회 조회가 limit을 초과하지 않는지 확인하는 테스트를 추가한다.

🤖 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/outbox/OutboxEventJpaRepository.java`
around lines 18 - 19, Update the OutboxEventJpaRepository.findPublishedBefore
query to accept a limit parameter and apply ORDER BY id ASC with a :limit bind
(e.g., findPublishedBefore(`@Param`("before") ZonedDateTime before,
`@Param`("limit") int limit)) and then change cleanupPublishedEvents to repeatedly
call findPublishedBefore(before, BATCH_SIZE) and delete each batch until an
empty list is returned so deletes run in bounded-size transactions; use the
existing BATCH_SIZE (50) constant and ensure the loop orders by id to make
chunked paging consistent, then add a test that simulates many PUBLISHED rows to
assert each fetch never returns more than BATCH_SIZE.
apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java-53-61 (1)

53-61: ⚠️ Potential issue | 🟠 Major

DLQ 전송 실패가 비동기 경로에서 누락될 수 있다

Line 53의 kafkaTemplate.send(dlqRecord)는 비동기이며, 현재 try-catch는 즉시 발생한 예외만 잡는다. 브로커 ACK 실패, 네트워크 오류, 타임아웃이 future completion에서 발생하면 로그 없이 누락되어, 실패 메시지 재처리 경로 자체가 유실된다. 현재 코드베이스의 OutboxRelayService에서 사용하는 것처럼 .get(timeout, TimeUnit.SECONDS)를 통해 동기 대기하거나, 최소한 whenComplete() 콜백으로 비동기 실패를 감지하고 로깅해야 한다. 수정 후에는 KafkaTemplate이 실패 future를 반환하는 케이스를 모킹해 에러 로그 및 후속 처리 분기가 실행되는 테스트를 추가해야 한다.

🔧 제안 수정안
-            kafkaTemplate.send(dlqRecord);
+            kafkaTemplate.send(dlqRecord).whenComplete((result, ex) -> {
+                if (ex != null) {
+                    log.error("[DLQ] 비동기 전송 실패 — topic={}, partition={}, offset={}",
+                            record.topic(), record.partition(), record.offset(), ex);
+                    return;
+                }
+                log.warn("[DLQ] 메시지 격리 — originalTopic={}, partition={}, offset={}, error={}",
+                        record.topic(), record.partition(), record.offset(), exception.getMessage());
+            });
-
-            log.warn("[DLQ] 메시지 격리 — originalTopic={}, partition={}, offset={}, error={}",
-                    record.topic(), record.partition(), record.offset(), exception.getMessage());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java`
around lines 53 - 61, The async send call in DlqPublisher
(kafkaTemplate.send(dlqRecord)) can fail on the returned Future after the
try-catch; change the handling to either block with the Future.get(timeout,
TimeUnit.SECONDS) or attach a completion handler (e.g., whenComplete/handle) to
detect broker ACK/network/timeouts and log errors (include exception stack/info)
and trigger any retry/compensating logic; update DlqPublisher to log failures
inside the completion callback and/or rethrow after get, and add unit tests that
mock kafkaTemplate.send to return a failed Future to assert the error logging
and subsequent behavior (similar to OutboxRelayService test pattern).
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventService.java-30-31 (1)

30-31: ⚠️ Potential issue | 🟠 Major

인프라 레이어의 @Transactional 선언을 제거하고 트랜잭션 경계를 애플리케이션 레이어로 통합해야 한다

Line 30의 @Transactional 선언은 infrastructure layer 서비스에서는 제거되어야 한다. OrderFacade와 PaymentFacade가 이미 @Transactional을 통해 트랜잭션을 관리하고 있으므로, OutboxEventService는 호출자의 트랜잭션에 자동으로 참여하며 별도 선언이 불필요하다. 주석에서 "호출자의 TX에 참여"한다고 명시했다면, 코드 차원의 명확성 위해 annotation을 제거하고 주석을 통해 "이 메서드는 호출자의 트랜잭션 컨텍스트 내에서만 호출되어야 한다"고 명시하는 것이 권장된다.

또한 Line 37-39에서 기술적 예외(JsonProcessingException)를 RuntimeException으로 감싸고 있는데, OrderFacade에서 사용하는 CoreException 같은 도메인 기반 예외 처리로 통일하면 호출자가 비즈니스 로직과 기술적 오류를 명확히 구분할 수 있다.

권장 수정 사항
  1. Line 30의 @Transactional 제거
  2. 클래스 수준 또는 메서드 주석에 "이 서비스는 호출자의 트랜잭션 내에서만 호출되어야 한다" 명시
  3. 예외 처리를 RuntimeException이 아닌 도메인 예외로 변경
  4. 통합 테스트에서 주문 생성 실패 시 주문 데이터와 Outbox 이벤트가 함께 롤백되는지 검증
🤖 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/outbox/OutboxEventService.java`
around lines 30 - 31, Remove the `@Transactional` annotation from
OutboxEventService.save so the infrastructure layer no longer defines
transaction boundaries (OrderFacade and PaymentFacade already manage
transactions); add a class- or method-level comment on OutboxEventService/ save
stating "This service must be called only within the caller's transaction
context" to make intent explicit; replace the current wrapping of
JsonProcessingException into RuntimeException with the domain-level exception
used by callers (e.g., throw CoreException or a specific OutboxDomainException)
so callers can distinguish business vs technical errors; and add/adjust
integration tests to assert that creating an order rolls back both order data
and OutboxEvent when the transaction fails.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java-89-98 (1)

89-98: ⚠️ Potential issue | 🟠 Major

markFailed()와 markRetry() 모두 retryCount를 증가시켜 이중 증가 문제가 있다.

OutboxRelayService.retryFailedEvents() 흐름에서:

  1. markRetry() 호출 → retryCount++
  2. Kafka 발행 실패 시 markFailed() 호출 → retryCount++

한 번의 재시도 실패에 retryCount가 2 증가한다. MAX_RETRY=5 설정 시 실제로는 2~3회만 재시도된다.

수정안: markRetry()에서 retryCount 증가 제거
 public void markRetry() {
     this.status = OutboxStatus.PENDING;
-    this.retryCount++;
 }

markRetry()는 상태만 변경하고, retryCount는 markFailed()에서만 증가하도록 책임을 명확히 분리한다.

추가 테스트: retryCount가 정확히 MAX_RETRY까지 증가하는지 검증하는 단위 테스트 추가 권장.

🤖 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/outbox/OutboxEventEntity.java`
around lines 89 - 98, The duplicate increment bug happens because both
OutboxEventEntity.markRetry() and markFailed() increase retryCount, causing two
increments per failed attempt in the OutboxRelayService.retryFailedEvents()
flow; to fix, remove the retryCount++ from markRetry() so markRetry() only sets
status to PENDING and leave retryCount++ solely in markFailed(), and then
add/update unit tests to assert that retryCount increments exactly once per
failed attempt up to MAX_RETRY; reference methods:
OutboxEventEntity.markRetry(), OutboxEventEntity.markFailed(), and
OutboxRelayService.retryFailedEvents().
apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java-66-76 (1)

66-76: ⚠️ Potential issue | 🟠 Major

DLQ 처리가 누락되어 장애 복구가 불가능하다.

CatalogMetricsConsumer는 처리 실패 시 DLQ로 전송하지만, 이 Consumer는 에러를 로깅만 한다. 처리 불가능한 메시지(잘못된 JSON, 시스템 장애 등)가 발생하면 해당 이벤트는 유실된다.

운영 관점에서 DLQ가 없으면 장애 분석 및 재처리가 어렵다.

수정안: DlqPublisher 의존성 추가
 public class CouponIssueConsumer {
     private final ObjectMapper objectMapper;
     private final CouponService couponService;
     private final CouponIssueRequestJpaRepository couponIssueRequestRepository;
     private final EventHandledJpaRepository eventHandledRepository;
+    private final DlqPublisher dlqPublisher;

     // ... constructor 수정

     public void consume(List<ConsumerRecord<Object, Object>> records, Acknowledgment ack) {
         for (ConsumerRecord<Object, Object> record : records) {
             try {
                 processRecord(record);
             } catch (Exception e) {
                 log.error("[CouponIssue] 처리 실패 — partition={}, offset={}, error={}",
                         record.partition(), record.offset(), e.getMessage(), e);
+                dlqPublisher.sendToDlq(record, e);
             }
         }
         ack.acknowledge();
     }
🤖 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/consumer/CouponIssueConsumer.java`
around lines 66 - 76, The consume method in CouponIssueConsumer currently only
logs failures, causing lost messages; inject a DlqPublisher dependency into
CouponIssueConsumer and, inside consume's exception catch block (where
processRecord(record) is called), publish the failed record payload and metadata
(partition/offset/exception message) to the DLQ via DlqPublisher before
continuing, so that malformed or unrecoverable messages can be debugged and
reprocessed; ensure you still acknowledge the batch (ack.acknowledge()) only
after publishing failures and include references to CouponIssueConsumer.consume
and processRecord when adding the DlqPublisher usage.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java-56-69 (1)

56-69: ⚠️ Potential issue | 🟠 Major

재시도 흐름에서 retryCount가 이중 증가하는 문제가 있다.

retryFailedEvents()에서 markRetry()를 호출하면 retryCount++가 실행되고, 이후 publishToKafka()에서 실패 시 markFailed()가 다시 retryCount++를 실행한다. 결과적으로 한 번의 재시도 실패에 retryCount가 2씩 증가하여, 실제로는 2~3번 재시도 후 MAX_RETRY(5)에 도달하게 된다.

운영 관점에서 재시도 횟수 추적이 부정확해지고, 의도한 5회 재시도가 보장되지 않는다.

수정안: markRetry()에서 retryCount 증가를 제거하거나, markFailed()에서 조건부 증가

Option 1: markRetry()에서 증가 제거 (OutboxEventEntity.java)

 public void markRetry() {
     this.status = OutboxStatus.PENDING;
-    this.retryCount++;
 }

Option 2: 재시도 흐름에서 markFailed() 대신 별도 메서드 사용

+public void markRetryFailed(String errorMessage) {
+    this.status = OutboxStatus.FAILED;
+    this.errorMessage = errorMessage;
+    // retryCount는 markRetry()에서 이미 증가함
+}

추가 테스트: 재시도 5회 후 MAX_RETRY 도달 확인 테스트 케이스 추가를 권장한다.

🤖 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/outbox/OutboxRelayService.java`
around lines 56 - 69, The retry flow increments retryCount twice because
retryFailedEvents() calls event.markRetry() (which increments retryCount) and
publishToKafka() on failure calls event.markFailed() (which also increments
retryCount); to fix, change the semantics so only failures increment the
counter: update OutboxEventEntity.markRetry() to stop incrementing retryCount
(only set status/nextAttempt metadata) and keep retryCount++ in markFailed(),
then ensure retryFailedEvents() still calls markRetry() before publishToKafka();
alternatively, if you prefer the opposite, remove retryCount++ from markFailed()
and only increment in markRetry()—apply the chosen single-source-of-increment
consistently across retryFailedEvents(), publishToKafka(), markRetry(), and
markFailed() and add/adjust tests to verify retryCount reaches MAX_RETRY after
five failed attempts.
🟡 Minor comments (3)
apps/commerce-api/src/main/java/com/loopers/application/product/ProductEventListener.java-28-34 (1)

28-34: ⚠️ Potential issue | 🟡 Minor

로그에 userId 출력 시 민감정보 노출 검토 필요

userId는 사용자 식별자로서 GDPR/CCPA 관점에서 PII(개인식별정보)에 해당할 수 있다. 운영 로그에 직접 출력 시 컴플라이언스 위반 가능성이 있다.

productId만 로깅하거나, userId를 마스킹/해시 처리하는 방안을 검토해야 한다.

🛡️ userId 마스킹 제안
     `@Async`
     `@EventListener`
     public void handleProductViewed(ProductViewedEvent event) {
-        log.info("[ProductEventListener] 상품 조회 — productId={}, userId={}",
-                event.productId(), event.userId());
+        log.info("[ProductEventListener] 상품 조회 — productId={}, hasUser={}",
+                event.productId(), event.userId() != null);
         // 추후: Kafka user-activity-events-v1 직접 발행
         // 또는: catalog-events-v1 직접 발행 (view_count 집계)
         // Outbox 없이 kafkaTemplate.send() — 유실 허용
     }

운영 환경 로그 수집 정책과 함께 민감정보 로깅 가이드라인 준수 여부를 확인해야 한다.

🤖 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/ProductEventListener.java`
around lines 28 - 34, The log in ProductEventListener.handleProductViewed
currently prints event.userId() which may expose PII; change the logging to
avoid raw userId by either logging only event.productId() or replacing
event.userId() with a masked/hashed value (e.g., call a new utility method
maskOrHashUserId(String userId) or hashUserId(String userId) and log its
result). Add the utility (e.g., maskOrHashUserId) near ProductEventListener or
in a shared util class and use event.productId() and the masked/hash output in
the log statement instead of the plain userId().
apps/commerce-api/build.gradle.kts-5-5 (1)

5-5: ⚠️ Potential issue | 🟡 Minor

modules:kafka testFixtures 의존성 추가 필요

kafka 모듈 의존성이 추가되었으나 testFixtures가 누락되었다. modules/kafka는 org.testcontainers:kafka를 포함한 testFixtures를 제공하고 있으며, 이미 추가된 modules:jpa, modules:redis와 동일하게 처리되어야 한다. 향후 kafka 관련 테스트 작성 시 컨테이너 기반의 테스트 환경을 지원하기 위해 다음을 추가한다:

testImplementation(testFixtures(project(":modules:kafka")))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/commerce-api/build.gradle.kts` at line 5, The build currently adds the
kafka module via implementation(project(":modules:kafka")) but omits the
testFixtures dependency; update the Gradle dependencies to also add the kafka
module's test fixtures by adding a testImplementation dependency that uses
testFixtures(project(":modules:kafka")) so that the modules/kafka
testcontainers-based fixtures are available for tests (mirror how modules:jpa
and modules:redis are handled).
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java-140-143 (1)

140-143: ⚠️ Potential issue | 🟡 Minor

동시 요청 시 getOrCreateMetrics에서 중복 INSERT 예외 발생 가능하다.

두 스레드가 동시에 같은 productId로 findById()를 호출하고 둘 다 empty를 받으면, 두 번의 save(ProductMetricsEntity.create(productId))가 실행되어 unique constraint violation이 발생할 수 있다.

수정안: UPSERT 또는 예외 처리 추가
 private ProductMetricsEntity getOrCreateMetrics(Long productId) {
-    return productMetricsRepository.findById(productId)
-            .orElseGet(() -> productMetricsRepository.save(ProductMetricsEntity.create(productId)));
+    return productMetricsRepository.findById(productId)
+            .orElseGet(() -> {
+                try {
+                    return productMetricsRepository.save(ProductMetricsEntity.create(productId));
+                } catch (DataIntegrityViolationException e) {
+                    // 동시 생성 경합 시 재조회
+                    return productMetricsRepository.findById(productId)
+                            .orElseThrow(() -> new IllegalStateException("ProductMetrics 생성 실패: " + productId));
+                }
+            });
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java`
around lines 140 - 143, getOrCreateMetrics can race: two threads may both see no
entity and attempt save, causing a unique-constraint insert error; fix by making
the create/save atomic or by handling the duplicate-insert exception and
retrying the lookup. Update getOrCreateMetrics (and related code paths using
productMetricsRepository and ProductMetricsEntity.create) to either call a
repository upsert method (implement a custom upsert query/DAO using SQL ON
CONFLICT / MERGE) or wrap the save in a try/catch for
DataIntegrityViolationException/DuplicateKeyException and on catch re-fetch with
findById() and return that result (optionally retry once), ensuring you return
the existing record instead of letting the exception propagate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c54349e-5b4b-49a0-b2cf-f48d99e27b85

📥 Commits

Reviewing files that changed from the base of the PR and between f9a398e and 2c0f1ae.

📒 Files selected for processing (40)
  • apps/commerce-api/build.gradle.kts
  • apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponFacade.java
  • apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java
  • apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java
  • apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.java
  • apps/commerce-api/src/main/java/com/loopers/application/product/ProductEventListener.java
  • apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
  • apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java
  • apps/commerce-api/src/main/java/com/loopers/config/KafkaTopicConfig.java
  • apps/commerce-api/src/main/java/com/loopers/domain/common/event/OrderConfirmedEvent.java
  • apps/commerce-api/src/main/java/com/loopers/domain/common/event/OrderItemSoldEvent.java
  • apps/commerce-api/src/main/java/com/loopers/domain/common/event/ProductLikedEvent.java
  • apps/commerce-api/src/main/java/com/loopers/domain/common/event/ProductViewedEvent.java
  • apps/commerce-api/src/main/java/com/loopers/domain/common/event/UserActivityEvent.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestStatus.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/event/EventHandledEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/event/EventHandledJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventService.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxStatus.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java
  • apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponFacadeAsyncIssueTest.java
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/event/EventHandledEntityTest.java
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxEventServiceTest.java
  • apps/commerce-api/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerTest.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/event/EventHandledEntity.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/event/EventHandledJpaRepository.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/event/IdempotencyService.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/product/ProductMetricsEntity.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/product/ProductMetricsJpaRepository.java
  • apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java
  • apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/DemoKafkaConsumer.java
  • modules/kafka/src/main/resources/kafka.yml
💤 Files with no reviewable changes (1)
  • apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/DemoKafkaConsumer.java

Comment on lines +130 to +138
private void handleOrderItemSold(JsonNode node) {
Long productId = node.path("productId").asLong();
int quantity = node.path("quantity").asInt(1);
ProductMetricsEntity metrics = getOrCreateMetrics(productId);
metrics.addSalesCount(quantity);
productMetricsRepository.save(metrics);
log.info("[CatalogMetrics] 판매량 집계 완료 — productId={}, salesCount={}",
productId, metrics.getSalesCount());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# OrderItemSoldEvent record 정의 확인
ast-grep --pattern $'record OrderItemSoldEvent($$$) {}'

# 또는 class 정의
rg -n "class OrderItemSoldEvent" --type=java -A 10

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

#!/bin/bash
# OrderItemSoldEvent를 더 광범위하게 검색
find . -type f -name "*.java" | xargs grep -l "OrderItemSoldEvent" | head -20

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 439


🏁 Script executed:

#!/bin/bash
# OrderFacade.java 찾기
find . -type f -name "OrderFacade.java"

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 165


🏁 Script executed:

#!/bin/bash
# CatalogMetricsConsumer.java의 전체 내용 확인
find . -type f -name "CatalogMetricsConsumer.java" -exec wc -l {} \; -exec cat -n {} \;

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 7411


🏁 Script executed:

#!/bin/bash
# OrderItemSoldEvent 정의 확인
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/common/event/OrderItemSoldEvent.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 530


🏁 Script executed:

#!/bin/bash
# OrderFacade.java 257-262 라인 확인
sed -n '250,270p' ./apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java | cat -n

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 1477


🏁 Script executed:

#!/bin/bash
# OrderFacade 전체에서 OrderItemSoldEvent 발행 부분 찾기
grep -n "OrderItemSoldEvent" ./apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java -B 5 -A 5

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 821


🏁 Script executed:

#!/bin/bash
# handleOrderItemSold 참조 찾기
grep -rn "handleOrderItemSold" ./apps/commerce-streamer/ --type=java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 207


🏁 Script executed:

#!/bin/bash
# OrderItemSoldEvent 관련 테스트 찾기
find . -type f -name "*Test.java" -o -name "*Tests.java" | xargs grep -l "OrderItemSoldEvent" 2>/dev/null

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 69


🏁 Script executed:

#!/bin/bash
# 카탈로그 메트릭 관련 테스트
find . -type f -name "*CatalogMetrics*Test*.java"

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


OrderItemSoldEvent 페이로드 구조와 불일치하여 판매량 집계가 실패한다.

OrderItemSoldEvent는 productQtyMap (Map<Long, Integer>) 필드를 포함하지만(OrderItemSoldEvent.java:13-16), 현재 코드는 존재하지 않는 productIdquantity 필드를 직접 추출하려 한다. 결과적으로 node.path("productId").asLong()은 0을, node.path("quantity").asInt(1)은 1을 반환하여 모든 판매가 잘못된 상품(productId=0)에 집계된다.

OrderFacade(257-262)에서 발행되는 페이로드:

{"orderId": 123, "productQtyMap": {"456": 2}}

현재 코드 결과: productId=0, quantity=1로 처리됨 → 판매 데이터 손실

수정안: productQtyMap 구조에 맞게 파싱
 private void handleOrderItemSold(JsonNode node) {
-    Long productId = node.path("productId").asLong();
-    int quantity = node.path("quantity").asInt(1);
-    ProductMetricsEntity metrics = getOrCreateMetrics(productId);
-    metrics.addSalesCount(quantity);
-    productMetricsRepository.save(metrics);
-    log.info("[CatalogMetrics] 판매량 집계 완료 — productId={}, salesCount={}",
-            productId, metrics.getSalesCount());
+    JsonNode productQtyMap = node.path("productQtyMap");
+    if (productQtyMap.isMissingNode() || !productQtyMap.isObject()) {
+        log.warn("[CatalogMetrics] productQtyMap 누락 — payload={}", node);
+        return;
+    }
+    productQtyMap.fields().forEachRemaining(entry -> {
+        Long productId = Long.parseLong(entry.getKey());
+        int quantity = entry.getValue().asInt(1);
+        ProductMetricsEntity metrics = getOrCreateMetrics(productId);
+        metrics.addSalesCount(quantity);
+        productMetricsRepository.save(metrics);
+        log.info("[CatalogMetrics] 판매량 집계 완료 — productId={}, salesCount={}",
+                productId, metrics.getSalesCount());
+    });
 }

추가 테스트: OrderItemSoldEvent 발행 시 payload를 로깅하고, 판매량 집계 전후 product_metrics를 검증하는 통합 테스트가 필요하다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java`
around lines 130 - 138, handleOrderItemSold currently reads missing fields
"productId" and "quantity"; instead parse the payload's productQtyMap field:
extract node.path("productQtyMap") as an object, iterate its field entries,
convert each field name to Long productId and its value to int quantity, call
getOrCreateMetrics(productId), metrics.addSalesCount(quantity),
productMetricsRepository.save(metrics) and log the result (use existing
getOrCreateMetrics, ProductMetricsEntity, productMetricsRepository.save and
log.info calls). Also add a debug log of the raw payload at the start of
handleOrderItemSold and add/adjust integration tests to publish an
OrderItemSoldEvent like {"orderId":123,"productQtyMap":{"456":2}} and assert
product_metrics is updated for productId=456.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

♻️ Duplicate comments (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java (1)

86-92: ⚠️ Potential issue | 🔴 Critical

@Transactional이 현재 처리 경로에 적용되지 않는다.

운영 영향: consume()processWithRetry()processRecord()가 같은 빈 내부 호출이라 Spring 프록시를 통과하지 않아, 쿠폰 발급·요청 상태 변경·event_handled 저장이 한 트랜잭션으로 묶이지 않는다.
수정안: processRecord()를 별도 @Service의 public 메서드로 분리하거나 TransactionTemplate으로 감싸 원자성을 보장해야 한다.
추가 테스트: 쿠폰 발급 후 eventHandledRepository.save() 직전에 예외를 주입했을 때 issued/request/event_handled가 모두 롤백되는지 검증이 필요하다.

Also applies to: 116-117

🤖 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/consumer/CouponIssueConsumer.java`
around lines 86 - 92, The current call chain consume() -> processWithRetry() ->
processRecord() invokes processRecord() as an internal private method so
`@Transactional` on it is not applied; refactor by extracting processRecord() into
a separate `@Service` with a public transactional method (e.g.,
CouponIssuanceService.issueCouponAndRecordEvent(...)) and call that from
processWithRetry()/consume(), or alternatively wrap the processRecord() logic in
a TransactionTemplate to ensure atomicity; ensure the transactional method
includes coupon issuance, request status update and
eventHandledRepository.save(...) so they commit/rollback together, and add a
unit/integration test that injects an exception just before
eventHandledRepository.save() to verify issued/request/event_handled are all
rolled back.
🤖 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/dlq/DlqPublisher.java`:
- Around line 74-80: The DLQ publisher currently swallows errors so sendToDlq()
in DlqPublisher.java must return a success indicator (boolean or Result) instead
of void and propagate failures; update sendToDlq() to catch, log, and return
false on failure (or throw a checked exception) and return true on success, then
modify CouponIssueConsumer.consume() to check the sendToDlq() result and, on DLQ
failure, avoid calling ack.acknowledge() and rethrow or propagate an exception
so the Kafka listener error handler (and ackMode) can retry or handle the
failure instead of committing the offset.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java`:
- Around line 124-135: The current CouponIssueConsumer logs the full payload
(variable payload) on JSON parse failures and missing required fields; replace
direct payload logging in methods handling parsing (e.g., where
log.error("[CouponIssue] JSON 파싱 실패 — payload={}", payload, e) and
log.warn("[CouponIssue] 필수 필드 누락 — payload={}", payload) are called) with a
sanitized log helper: implement a sanitizePayload(String) utility that returns
partition/offset info if available, payload length, a stable hash, and masked or
truncated values for identifiers (eventId, requestId, userId) instead of the raw
JSON; update the logging calls to use sanitizePayload(payload) and add unit
tests for sanitizePayload to verify behavior with very long payloads and
payloads containing identifiers to ensure masking/truncation and hash generation
are correct.
- Around line 153-170: The current catch-all in CouponIssueConsumer around
couponService.issue / couponIssueRequestRepository.save /
eventHandledRepository.save incorrectly wraps all Exceptions as
BusinessFailureException, preventing transient infra errors from being retried;
change the error handling to catch business errors (e.g., CoreException)
explicitly, perform request.markFailed(...), persist via
couponIssueRequestRepository.save(request) and eventHandledRepository.save(...),
then wrap/throw BusinessFailureException as before, but do NOT catch generic
Exception or infra exceptions (e.g., DataAccessException) — let them propagate
(or rethrow) so processWithRetry() can retry/DLQ them; update the try/catch
block to reference CouponIssueConsumer, couponService.issue, request,
couponIssueRequestRepository, eventHandledRepository, BusinessFailureException,
CoreException, DataAccessException and ensure tests cover CoreException→FAILED
and DataAccessException→retry path.
- Around line 121-149: The consumer currently returns quietly on JSON parse
errors, missing fields, or missing request entities which causes bad messages to
be acked; change objectMapper.readTree(...) parsing catch, the
eventId/requestId/templateId/userId extraction logic, and the
couponIssueRequestRepository.findById(...) null path to throw a unified
InvalidMessageException (or the app's DLQ-triggering exception) so malformed
JSON, any missing/zero eventId/requestId/templateId/userId, and non-existent
requestId are routed to DLQ, while keeping the existing behavior of silently
returning when eventHandledRepository.existsByEventId(eventId) indicates a
duplicate; ensure all four fields are validated (no null/zero) before
proceeding.
- Around line 63-77: In consume(List<ConsumerRecord<Object, Object>> records,
Acknowledgment ack) the InterruptedException handler currently restores the
interrupt flag but lets the loop continue and always calls ack.acknowledge(),
causing lost records; change the handler so that when InterruptedException is
caught from processWithRetry(record) you call dlqPublisher.sendToDlq(record, e,
0) for that record and then stop processing the batch (either break/return from
consume or rethrow the InterruptedException) so ack.acknowledge() is not
executed for a partially processed batch; ensure any other Exception handling
remains unchanged and that only fully-handled-or-DLQ'ed batches trigger
ack.acknowledge().

In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java`:
- Around line 25-30: DlqPublisher currently hardcodes DLQ_TOPIC
("pipeline-dlq-v1") which can cause publishes to fail when the topic isn't
created by other services; change DlqPublisher to read the DLQ topic name from
configuration (inject a property, e.g. dlq.topic) instead of DLQ_TOPIC constant,
and remove the hardcoded string; ensure topic creation is performed in a
shared/common module or add a topic-creation bean in the streamer context
(mirror the logic in KafkaTopicConfig) so the topic exists even when streamer is
deployed alone; update tests to use Testcontainers Kafka with auto-create
disabled to verify DlqPublisher.publish flows to the configured topic.
- Around line 73-80: sendToDlq currently swallows
ExecutionException/TimeoutException/InterruptedException and always allows
CatalogMetricsConsumer to call ack.acknowledge(), causing message loss; change
sendToDlq (in DlqPublisher) to surface failures (either return boolean success
or rethrow a checked exception) so the caller can decide ack vs nack, ensure
InterruptedException resets the interrupt flag and is propagated as failure, and
update CatalogMetricsConsumer to check the boolean or catch the exception before
calling ack.acknowledge() so that DLQ send failures (including
TimeoutException/InterruptedException) result in not acknowledging the original
message and are covered by tests that simulate KafkaTemplate.send(...).get()
throwing those exceptions.

---

Duplicate comments:
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java`:
- Around line 86-92: The current call chain consume() -> processWithRetry() ->
processRecord() invokes processRecord() as an internal private method so
`@Transactional` on it is not applied; refactor by extracting processRecord() into
a separate `@Service` with a public transactional method (e.g.,
CouponIssuanceService.issueCouponAndRecordEvent(...)) and call that from
processWithRetry()/consume(), or alternatively wrap the processRecord() logic in
a TransactionTemplate to ensure atomicity; ensure the transactional method
includes coupon issuance, request status update and
eventHandledRepository.save(...) so they commit/rollback together, and add a
unit/integration test that injects an exception just before
eventHandledRepository.save() to verify issued/request/event_handled are all
rolled back.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b38974f0-4a1f-4c44-a766-273a523ef892

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0f1ae and 40da025.

📒 Files selected for processing (4)
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java
  • apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java
  • apps/commerce-api/src/test/java/com/loopers/concurrency/KafkaCouponConcurrencyTest.java
  • apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java
✅ Files skipped from review due to trivial changes (1)
  • apps/commerce-api/src/test/java/com/loopers/concurrency/KafkaCouponConcurrencyTest.java

Comment on lines +74 to +80
} catch (ExecutionException | TimeoutException e) {
log.error("[DLQ] DLQ 전송 실패! 메시지 유실 위험 — topic={}, partition={}, offset={}, error={}",
record.topic(), record.partition(), record.offset(), e.getMessage(), e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
log.error("[DLQ] DLQ 전송 중단 — topic={}, partition={}, offset={}",
record.topic(), record.partition(), record.offset());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's read the complete DlqPublisher.java file
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 4432


🏁 Script executed:

# Search for all calls to sendToDlq method
rg "sendToDlq" apps/commerce-api/src --type java -B 2 -A 2

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 2037


🏁 Script executed:

# Check if there are any tests for DlqPublisher
fd -e java "DlqPublisher" apps/commerce-api/src/test --type f

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# Let's see the complete context of CouponIssueConsumer to understand offset management
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java | head -150

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 7119


🏁 Script executed:

# Search for offset management or kafka listener configuration
rg "offset|commit|@KafkaListener" apps/commerce-api/src/main/java/com/loopers/interfaces/consumer --type java -B 2 -A 2

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 3585


DLQ 전송 실패가 묵음처리되어 offset이 정상 커밋되면 메시지가 영구 유실된다.

운영 위험: sendToDlq()은 void 반환형이고 예외를 로깅만 하므로, 호출부인 CouponIssueConsumer.consume()은 DLQ 적재 성공 여부를 알 수 없다. 현재 구조상 DLQ 전송이 ExecutionException, TimeoutException, InterruptedException으로 실패해도 line 77의 ack.acknowledge()가 실행되어 offset이 정상 커밋된다. 결과적으로 메시지는 쿠폰 처리 실패로 인해 Kafka consumer에서 버려지고, 동시에 DLQ 전송도 실패하여 진자 영구 유실된다.

수정안: sendToDlq()의 반환형을 boolean 또는 Result 타입으로 변경하여 성공/실패를 명시적으로 반환한다. CouponIssueConsumer.consume()에서 DLQ 전송 실패 시 ack.acknowledge() 호출을 중단하고 예외를 전파하여 Kafka listener의 에러 처리기가 동작하도록 한다.

검증 항목: DLQ 전송이 실패했을 때 consumer가 offset을 커밋하지 않는지, Kafka listener 설정(ackMode)이 적절히 처리하는지 확인한다.

🤖 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/dlq/DlqPublisher.java`
around lines 74 - 80, The DLQ publisher currently swallows errors so sendToDlq()
in DlqPublisher.java must return a success indicator (boolean or Result) instead
of void and propagate failures; update sendToDlq() to catch, log, and return
false on failure (or throw a checked exception) and return true on success, then
modify CouponIssueConsumer.consume() to check the sendToDlq() result and, on DLQ
failure, avoid calling ack.acknowledge() and rethrow or propagate an exception
so the Kafka listener error handler (and ackMode) can retry or handle the
failure instead of committing the offset.

Comment on lines +63 to +77
public void consume(List<ConsumerRecord<Object, Object>> records, Acknowledgment ack) {
for (ConsumerRecord<Object, Object> record : records) {
try {
processWithRetry(record);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
log.error("[CouponIssue] 처리 중단 — partition={}, offset={}",
record.partition(), record.offset());
} catch (Exception e) {
log.error("[CouponIssue] 예외 → DLQ — partition={}, offset={}, error={}",
record.partition(), record.offset(), e.getMessage(), e);
dlqPublisher.sendToDlq(record, e, 0);
}
}
ack.acknowledge();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

fd -t f "CouponIssueConsumer.java" --exec cat -n {} \;

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 8514


🏁 Script executed:

# Verify the exception handling pattern and check if DlqPublisher is properly handling the record
rg -A 3 "sendToDlq" apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 284


🏁 Script executed:

# Check if InterruptedException handling pattern is used elsewhere in similar consumers
fd -t f "Consumer.java" --path "*/interfaces/consumer/*" -x grep -l "InterruptedException" {} \;

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 318


🏁 Script executed:

# Verify if there's a finally block or additional logic that might affect ack behavior
sed -n '63,80p' apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 861


인터럽트 발생 시 배치를 무조건 ack하므로 미처리 레코드가 유실된다.

문제점: InterruptedException이 발생하면 67~70번 줄에서 인터럽트 플래그를 복원하고 로깅하지만, breakreturn이 없어서 루프가 계속 진행된다. 그 결과 77번 줄의 ack.acknowledge()가 항상 실행되어, 중단된 레코드는 DLQ로 전송되지 않고 배치 전체가 커밋된다. 이 레코드는 영구히 재전달되지 않는다.

운영 영향: processWithRetry()의 지수 백오프 Thread.sleep()에서 발생한 InterruptedException은 중단된 처리를 의도하지만, 현재 코드는 해당 레코드를 DLQ에 적재하지 않고 조용히 ack함으로써 데이터 손실을 초래한다.

수정안:

  • InterruptedException 발생 시 해당 레코드를 DLQ로 전송한 후 루프를 중단하거나, 예외를 상위로 전파하여 컨테이너가 배치 전체를 재처리하도록 위임해야 한다.
  • 또는 ack.acknowledge()를 조건부로 변경하여, 모든 레코드가 성공하거나 DLQ 적재된 경우에만 호출해야 한다.

추가 테스트: processWithRetry()에서 Thread.sleep() 중 interrupt 발생 시, ack.acknowledge()가 호출되지 않거나 해당 레코드가 DLQ에 적재되는지 검증한다.

🤖 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/consumer/CouponIssueConsumer.java`
around lines 63 - 77, In consume(List<ConsumerRecord<Object, Object>> records,
Acknowledgment ack) the InterruptedException handler currently restores the
interrupt flag but lets the loop continue and always calls ack.acknowledge(),
causing lost records; change the handler so that when InterruptedException is
caught from processWithRetry(record) you call dlqPublisher.sendToDlq(record, e,
0) for that record and then stop processing the batch (either break/return from
consume or rethrow the InterruptedException) so ack.acknowledge() is not
executed for a partially processed batch; ensure any other Exception handling
remains unchanged and that only fully-handled-or-DLQ'ed batches trigger
ack.acknowledge().

Comment on lines +73 to +80
} catch (ExecutionException | TimeoutException e) {
log.error("[DLQ] DLQ 전송 실패! 메시지 유실 위험 — topic={}, partition={}, offset={}, error={}",
record.topic(), record.partition(), record.offset(), e.getMessage(), e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
log.error("[DLQ] DLQ 전송 중단 — topic={}, partition={}, offset={}",
record.topic(), record.partition(), record.offset());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "DlqPublisher.java" -type f

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 255


🏁 Script executed:

cat -n ./apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 4359


🏁 Script executed:

rg "sendToDlq" --type java -B 5 -A 5 ./apps/commerce-streamer/

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 3417


🏁 Script executed:

cat -n ./apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogMetricsConsumer.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 7308


🏁 Script executed:

find ./apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer -name "*Consumer.java" -type f

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 183


DLQ 전송 실패를 예외 처리로 삼키면 메시지가 유실된다.

운영 영향: sendToDlq() 메서드가 void를 반환하고 ExecutionException, TimeoutException, InterruptedException을 모두 로그만 남기므로, DLQ 전송이 실패해도 호출부인 CatalogMetricsConsumer는 이를 감지할 수 없다. 결과적으로 라인 69의 ack.acknowledge()가 무조건 실행되어 메시지가 확실하게 손실된다.

수정안: sendToDlq() 메서드가 boolean 또는 예외를 반환하도록 변경하고, 호출부에서 DLQ 전송 성공 여부를 확인한 후 ack 또는 nack을 결정해야 한다. TimeoutExceptionInterruptedException 발생 시에도 호출부가 처리 실패로 인식할 수 있어야 한다.

추가 테스트: KafkaTemplate.send(...).get()TimeoutExceptionInterruptedException을 던질 때 호출부가 성공 처리하지 않음을 검증해야 한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/dlq/DlqPublisher.java`
around lines 73 - 80, sendToDlq currently swallows
ExecutionException/TimeoutException/InterruptedException and always allows
CatalogMetricsConsumer to call ack.acknowledge(), causing message loss; change
sendToDlq (in DlqPublisher) to surface failures (either return boolean success
or rethrow a checked exception) so the caller can decide ack vs nack, ensure
InterruptedException resets the interrupt flag and is propagated as failure, and
update CatalogMetricsConsumer to check the boolean or catch the exception before
calling ack.acknowledge() so that DLQ send failures (including
TimeoutException/InterruptedException) result in not acknowledging the original
message and are covered by tests that simulate KafkaTemplate.send(...).get()
throwing those exceptions.

@iohyeon iohyeon changed the title [Volume-7] 임나현 [Volume-7] ApplicationEvent 경계 분리 + Kafka Outbox Pipeline + 선착순 쿠폰 - 임나현 Mar 26, 2026
  FOR UPDATE SKIP LOCKED가 작동하지 않던 버그 수정:
  - JPQL에서 Native Query로 변경하여 SKIP LOCKED 활성화
  - PROCESSING 상태 단위 테스트 5개 추가
  - 멀티 스레드 통합 테스트로 중복 방지 검증
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

♻️ Duplicate comments (3)
apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java (3)

89-118: ⚠️ Potential issue | 🟠 Major

비정상 메시지를 return으로 끝내면 ACK되어 영구 유실된다.

JSON 파싱 실패, eventId/requestId/templateId/userId 누락 또는 0값, 존재하지 않는 requestId가 모두 return으로 끝나서 배치 마지막 ACK에 포함된다. 운영에서는 잘못된 메시지가 DLQ로 격리되지 않고 조용히 사라지며, 현재 로그는 payload 원문까지 남겨 식별자 노출 위험도 만든다. 이 경로는 공통 invalid-message 예외로 올려 ErrorHandler/DLQ로 보내고, 로그는 길이 제한과 마스킹이 적용된 요약만 남기는 편이 안전하다. 추가 테스트로 malformed JSON, 각 필드 누락/0값, 존재하지 않는 requestId가 모두 DLQ 경로로 가고 로그에 raw payload가 남지 않는지 검증이 필요하다. As per coding guidelines "로깅 시 민감정보 노출 가능성을 점검한다."

🤖 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/consumer/CouponIssueConsumer.java`
around lines 89 - 118, The handler in CouponIssueConsumer should not swallow
invalid messages with plain returns; instead throw a specific
InvalidMessageException (or reuse the app's common invalid-message exception)
from the parsing and validation branches in the method that processes payloads
(the JSON read/field extraction and request lookup blocks) so the
ErrorHandler/DLQ handles them; also change log.error/log.warn calls in those
branches to log a short, masked summary (limit length and mask identifiers)
rather than the raw payload; add unit/integration tests asserting that malformed
JSON, missing/zero eventId/requestId/templateId/userId, and non-existent
requestId result in the InvalidMessageException path (DLQ) and that logs do not
contain the full raw payload.

121-139: ⚠️ Potential issue | 🔴 Critical

일시적 인프라 오류까지 비즈니스 실패로 확정하면 회복 기회가 사라진다.

지금 catch (Exception)couponService.issue(), couponIssueRequestRepository.save(), eventHandledRepository.save()에서 난 일시적 DB/인프라 오류까지 모두 FAILED + event_handled로 확정한다. 이렇게 되면 순간 장애 1회로 요청이 영구 실패로 굳고, 다음 재전달에서도 event_handled 때문에 스킵된다. 비즈니스 예외만 별도로 잡아 FAILED를 기록하고, 그 외 인프라 예외는 그대로 던져 ErrorHandler/DLQ 경로에 맡기는 편이 맞다. 추가 테스트로 비즈니스 실패는 FAILED로 남고, DataAccessException 같은 인프라 오류는 요청 상태와 event_handled를 남기지 않은 채 재시도/DLQ로 가는지 나눠서 검증해 달라.

🤖 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/consumer/CouponIssueConsumer.java`
around lines 121 - 139, The current broad catch in CouponIssueConsumer causes
transient infra errors to be treated as business failures; change the error
handling to only catch and handle domain/business exceptions (e.g., the specific
business exception type thrown by couponService.issue — replace catch
(Exception) with catch (YourBusinessExceptionType) or catch
BusinessFailureException if that is the domain type) and inside that block
perform request.markFailed(...), couponIssueRequestRepository.save(request),
eventHandledRepository.save(...), and rethrow/wrap as BusinessFailureException
as needed; do not swallow or record event_handled for other exceptions (e.g.,
DataAccessException, network/infra errors) — let them propagate to
ErrorHandler/DLQ. Add tests to verify business failures record FAILED +
event_handled while DataAccessException-like infra errors do not persist request
state or event_handled and are retried/DLQ'd.

67-70: ⚠️ Potential issue | 🔴 Critical

내부 호출 때문에 @Transactional이 적용되지 않는다.

consume()가 같은 빈의 processRecord()를 직접 호출해서 트랜잭션 프록시를 타지 않는다. 운영에서는 쿠폰 발급, 요청 상태 업데이트, event_handled 저장 사이에서 중간 실패가 나면 상태가 갈라지고 부분 반영이 남는다. 별도 processor @Service로 분리해 프록시 경유로 호출하고, 비즈니스 실패를 예외로 계속 표현할 계획이면 FAILED 상태 저장이 롤백되지 않도록 noRollbackFor = BusinessFailureException.class 또는 명시적 결과 객체로 흐름을 분리하는 편이 안전하다. 추가 테스트로 발급 성공 후 event_handled 저장 직전에 예외를 강제로 발생시켜 전체가 함께 롤백되거나, 비즈니스 실패만 예외 없이 확정 저장되는지 검증이 필요하다.

Spring Framework `@Transactional` self invocation proxy mode official documentation noRollbackFor RuntimeException

Also applies to: 85-86

🤖 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/consumer/CouponIssueConsumer.java`
around lines 67 - 70, 현재 consume()가 같은 빈의 processRecord()를 내부 호출해 트랜잭션 프록시를 거치지
않아 `@Transactional` 효과가 적용되지 않습니다; 분리해 트랜잭션 범위를 확실히 하려면 processRecord()의 로직을 새로운
`@Service` 클래스(예: CouponProcessor)로 옮기고 consume()에서는 이 서비스의 메서드를 주입받아 호출하도록 변경하세요;
또한 비즈니스 실패를 예외로 표현하면서 FAILED 상태가 롤백되지 않게 하려면 새로운 service 메서드의 `@Transactional에`
noRollbackFor = BusinessFailureException.class를 적용하거나, 예외 대신 명시적 결과 객체로 성공/실패
흐름을 분리해 event_handled 저장과 상태 업데이트가 원하는 방식으로 영속화되도록 테스트(발급 성공 후 event_handled 직전
강제 예외 등)를 추가해 검증하세요.
🧹 Nitpick comments (1)
apps/commerce-api/src/main/java/com/loopers/config/TopicProperties.java (1)

29-43: 예외 메시지에 실제 값 포함 권장

운영 환경에서 설정 오류 발생 시, 현재 예외 메시지만으로는 어떤 값이 문제인지 파악하기 어렵다. 로그 분석 및 장애 대응 시간 단축을 위해 실제 입력값을 메시지에 포함하는 것이 좋다.

♻️ 예외 메시지 개선 예시
         public TopicConfig {
             if (name == null || name.isBlank()) {
-                throw new IllegalArgumentException("Topic name must not be blank");
+                throw new IllegalArgumentException("Topic name must not be blank: received=" + name);
             }
             if (partitions <= 0) {
-                throw new IllegalArgumentException("Partitions must be positive");
+                throw new IllegalArgumentException("Partitions must be positive: received=" + partitions);
             }
             if (replicas <= 0) {
-                throw new IllegalArgumentException("Replicas must be positive");
+                throw new IllegalArgumentException("Replicas must be positive: received=" + replicas);
             }
             if (minInsyncReplicas <= 0 || minInsyncReplicas > replicas) {
                 throw new IllegalArgumentException(
-                    "min-insync-replicas must be positive and <= replicas"
+                    "min-insync-replicas must be positive and <= replicas: minInsyncReplicas=" + minInsyncReplicas + ", replicas=" + replicas
                 );
             }
         }
🤖 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/config/TopicProperties.java`
around lines 29 - 43, The current TopicConfig compact constructor throws generic
IllegalArgumentException messages; update each check in the TopicConfig
constructor (for name, partitions, replicas, minInsyncReplicas) to include the
offending value(s) in the exception text (e.g., include name, partitions,
replicas, and minInsyncReplicas) so the thrown IllegalArgumentException for
TopicConfig contains both the descriptive message and the actual value(s) that
failed validation.
🤖 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/config/TopicProperties.java`:
- Around line 15-21: TopicProperties 레코드에 compact constructor를 추가해
catalogEvents, orderEvents, couponIssueRequests, userActivityEvents, pipelineDlq
필드를 null 검사하도록 하세요; 누락된 YAML 바인딩으로 null이 들어오면 즉시 IllegalArgumentException(또는
NullPointerException 대신 명확한 메시지)을 던져 KafkaTopicConfig 등에서 발생할 NPE를 방지하고 디버깅용
메시지를 포함하세요; 변경 후 설정 누락 시나리오를 검증하는 단위 테스트(TopicProperties 생성 시 null 입력 시 예외 발생)를
추가하세요.
- Around line 27-28: TopicProperties 생성자에서 전달된 retentionMs에 대한 유효성 검증이 빠져 있어 0
또는 음수값이 들어갈 경우 문제가 발생하므로, TopicProperties(...) 생성자(또는 빌더 초기화 지점)에서 retentionMs가
null이 아니면 반드시 > 0인지 검사하고 그렇지 않으면 IllegalArgumentException을 던지도록 추가하세요; 검증 로직은
TopicProperties 클래스 내부(예: validateRetentionMs 메서드 또는 생성자 시작부)로 넣고 메시지에 필드명과
유효조건을 포함하여 명확히 하며, 이 동작을 보장하는 단위테스트를 추가하여 retentionMs == 0 및 retentionMs < 0
케이스에서 예외가 발생하는지 확인하세요.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java`:
- Around line 95-104: markRetry() currently increments retryCount causing
double-counting alongside markFailed(); update OutboxEventEntity so markRetry()
only sets status = OutboxStatus.PENDING (and does not modify retryCount) while
markFailed(String errorMessage) remains responsible for incrementing retryCount
and setting errorMessage/status, and add/adjust unit tests for OutboxEventEntity
to simulate repeated failures to assert that retryCount increases only once per
failure and that retries stop after the configured limit.
- Around line 22-25: Remove the duplicate composite index on (status,
created_at) by deleting one of the two index annotations (either remove
`@Index`(name = "idx_outbox_pending", columnList = "status, created_at", ...) or
remove `@Index`(name = "idx_outbox_status_created", columnList = "status,
created_at")), and instead add a migration that creates the correct index needed
by cleanupPublishedEvents() (e.g., a new index on (status, published_at) or a
partial index for published rows) so writes are not amplified; update or add a
schema-validation test and/or an EXPLAIN-based test to verify only the intended
indexes exist and are used at runtime.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java`:
- Around line 135-143: The cleanupPublishedEvents method currently loads all
entities via findPublishedBefore(cutoff) and calls
outboxEventJpaRepository.deleteAll(oldEvents), which can OOM and cause huge
transactions; change it to delete in bounded batches: implement a batch-delete
loop in cleanupPublishedEvents that repeatedly queries only IDs (or a Pageable
of IDs) from outboxEventJpaRepository.findPublishedBefore(cutoff,
Pageable.ofSize(batchSize)) or a custom repository method
findIdsPublishedBefore(cutoff, limit) and then call a bulk delete like
outboxEventJpaRepository.deleteAllByIdInBatch(idList) or a JPQL/SQL delete where
id IN (:ids); commit each batch (remove the big `@Transactional` scope or use a
new transaction per batch) until no more rows remain; add an integration test
that inserts >batchSize published rows and asserts the method deletes them
across multiple batches.
- Around line 88-117: The publishProcessingEvents() method can read the same
PROCESSING rows on multiple instances—add a lease/owner-claim step so Phase 2
only publishes rows this instance has claimed. Implement a new transactional
claim method (e.g., claimProcessingEvents(ownerId, leaseUntil, batchSize)) that
does an UPDATE ... WHERE status=PROCESSING AND (owner IS NULL OR leaseUntil <
now) LIMIT BATCH_SIZE SET owner = :ownerId, leaseUntil = :expiry and returns the
claimed outboxIds, then fetch those OutboxEventEntity rows and have
publishProcessingEvents() call publishToKafka only for those owned rows and
later set status to PUBLISHED/FAILED inside the same/another transaction;
reference functions/classes: publishProcessingEvents(),
outboxEventJpaRepository.findProcessingEvents (replace with
claimProcessingEvents), OutboxEventEntity fields owner/leaseUntil,
publishToKafka, and transactionTemplate. Also add a concurrent integration test
that boots two relay instances and asserts each outboxId is published exactly
once.
- Around line 120-132: retryFailedEvents currently runs the synchronous Kafka
send inside a long `@Transactional` method causing DB locks/connection holds;
change the flow so you: 1) open a short transaction to call
outboxEventJpaRepository.findRetryableEvents(...) and update each entity via
OutboxEventEntity.markRetry() and persist those changes, then commit; 2) perform
publishToKafka(OutboxEventEntity) calls outside any `@Transactional` boundary
(ensure publishToKafka still enforces send timeouts and does not block
indefinitely); 3) for each publish result, open a new short transaction to
update the entity status (e.g., markSent/markFailed) and save it; update
retryFailedEvents() to remove the class-level `@Transactional` and orchestrate
these short transactions, and add a test double for Kafka send that simulates a
timeout to verify other selectors aren’t blocked.

In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxEventServiceTest.java`:
- Around line 111-145: Add an integration-style unit test that exercises the
full FAILED -> markRetry() -> re-publish failure cycle instead of only calling
markFailed() and then querying findRetryableEvents; create an OutboxEventEntity
via outboxEventService.save, then call the OutboxRelayService.retryFailedEvents
(or replicate its state transitions by invoking the same
markRetry()/publish-failure path) repeatedly to simulate consecutive publish
failures, saving the entity each step, and assert that
findRetryableEvents(maxRetry, batchSize) includes the event up until the
configured maxRetry and excludes it immediately after the final failed retry;
reference OutboxEventEntity.markFailed, OutboxEventEntity.markRetry (or the
retry path inside OutboxRelayService.retryFailedEvents),
outboxEventJpaRepository.findRetryableEvents and outboxEventService.save to
locate the code to extend.

In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxRelayServiceIntegrationTest.java`:
- Around line 230-231: Remove the flaky fairness assertion in
OutboxRelayServiceIntegrationTest that checks
acquiredCountByThread.values().allMatch(count -> count > 0); instead assert only
that the total number of acquired messages equals the expected total and that
there are no duplicate message IDs across threads (use acquiredCountByThread and
the collection of acquiredIds to verify uniqueness and sum), and optionally wrap
the test body in a short loop to repeat the scenario multiple times to ensure
stability; if fairness is required later, implement a deterministic shard/lease
policy in the production code and add dedicated tests for that policy rather
than asserting fairness here.

In `@modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java`:
- Around line 75-77: The batch listener currently causes whole-batch DLQ
recovery because CouponIssueConsumer.consume() throws a generic Exception and
DefaultErrorHandler lacks record index; update error handling so failures
include the failed record index (throw new BatchListenerFailedException("...",
recordIndex) from CouponIssueConsumer.consume()) or convert the listener to a
record-level listener and configure the factory's error handler (where
factory.setCommonErrorHandler(createErrorHandler(kafkaTemplate)) is set) to use
DeadLetterPublishingRecoverer so only the single failed record is sent to DLQ;
after changes, add an end-to-end test that simulates batch size 3 with only the
2nd record failing and assert that records 1 and 3 are not sent to DLQ.

---

Duplicate comments:
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java`:
- Around line 89-118: The handler in CouponIssueConsumer should not swallow
invalid messages with plain returns; instead throw a specific
InvalidMessageException (or reuse the app's common invalid-message exception)
from the parsing and validation branches in the method that processes payloads
(the JSON read/field extraction and request lookup blocks) so the
ErrorHandler/DLQ handles them; also change log.error/log.warn calls in those
branches to log a short, masked summary (limit length and mask identifiers)
rather than the raw payload; add unit/integration tests asserting that malformed
JSON, missing/zero eventId/requestId/templateId/userId, and non-existent
requestId result in the InvalidMessageException path (DLQ) and that logs do not
contain the full raw payload.
- Around line 121-139: The current broad catch in CouponIssueConsumer causes
transient infra errors to be treated as business failures; change the error
handling to only catch and handle domain/business exceptions (e.g., the specific
business exception type thrown by couponService.issue — replace catch
(Exception) with catch (YourBusinessExceptionType) or catch
BusinessFailureException if that is the domain type) and inside that block
perform request.markFailed(...), couponIssueRequestRepository.save(request),
eventHandledRepository.save(...), and rethrow/wrap as BusinessFailureException
as needed; do not swallow or record event_handled for other exceptions (e.g.,
DataAccessException, network/infra errors) — let them propagate to
ErrorHandler/DLQ. Add tests to verify business failures record FAILED +
event_handled while DataAccessException-like infra errors do not persist request
state or event_handled and are retried/DLQ'd.
- Around line 67-70: 현재 consume()가 같은 빈의 processRecord()를 내부 호출해 트랜잭션 프록시를 거치지
않아 `@Transactional` 효과가 적용되지 않습니다; 분리해 트랜잭션 범위를 확실히 하려면 processRecord()의 로직을 새로운
`@Service` 클래스(예: CouponProcessor)로 옮기고 consume()에서는 이 서비스의 메서드를 주입받아 호출하도록 변경하세요;
또한 비즈니스 실패를 예외로 표현하면서 FAILED 상태가 롤백되지 않게 하려면 새로운 service 메서드의 `@Transactional에`
noRollbackFor = BusinessFailureException.class를 적용하거나, 예외 대신 명시적 결과 객체로 성공/실패
흐름을 분리해 event_handled 저장과 상태 업데이트가 원하는 방식으로 영속화되도록 테스트(발급 성공 후 event_handled 직전
강제 예외 등)를 추가해 검증하세요.

---

Nitpick comments:
In `@apps/commerce-api/src/main/java/com/loopers/config/TopicProperties.java`:
- Around line 29-43: The current TopicConfig compact constructor throws generic
IllegalArgumentException messages; update each check in the TopicConfig
constructor (for name, partitions, replicas, minInsyncReplicas) to include the
offending value(s) in the exception text (e.g., include name, partitions,
replicas, and minInsyncReplicas) so the thrown IllegalArgumentException for
TopicConfig contains both the descriptive message and the actual value(s) that
failed validation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 36f252eb-32de-4922-90db-ee1c98482fae

📥 Commits

Reviewing files that changed from the base of the PR and between f760b62 and e0230cc.

📒 Files selected for processing (12)
  • apps/commerce-api/src/main/java/com/loopers/config/KafkaTopicConfig.java
  • apps/commerce-api/src/main/java/com/loopers/config/TopicProperties.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxStatus.java
  • apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java
  • apps/commerce-api/src/main/resources/application-local.yml
  • apps/commerce-api/src/main/resources/application-prd.yml
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxEventServiceTest.java
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxRelayServiceIntegrationTest.java
  • modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java
✅ Files skipped from review due to trivial changes (2)
  • apps/commerce-api/src/main/resources/application-prd.yml
  • apps/commerce-api/src/main/resources/application-local.yml
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/commerce-api/src/main/java/com/loopers/config/KafkaTopicConfig.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java

Comment on lines +15 to +21
public record TopicProperties(
TopicConfig catalogEvents,
TopicConfig orderEvents,
TopicConfig couponIssueRequests,
TopicConfig userActivityEvents,
TopicConfig pipelineDlq
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

외부 레코드에서 중첩 config 객체의 null 검증 누락

TopicProperties 레코드에 compact constructor가 없어 YAML 설정이 누락된 경우 catalogEvents, orderEvents 등이 null로 바인딩될 수 있다. 이후 KafkaTopicConfig에서 해당 객체 접근 시 NPE가 발생하여 애플리케이션 기동 실패로 이어진다. 장애 발생 시 원인 파악이 어려워지므로 명시적 검증이 필요하다.

🛡️ null 검증 추가 제안
 `@ConfigurationProperties`(prefix = "kafka.topic")
 public record TopicProperties(
         TopicConfig catalogEvents,
         TopicConfig orderEvents,
         TopicConfig couponIssueRequests,
         TopicConfig userActivityEvents,
         TopicConfig pipelineDlq
 ) {
+    public TopicProperties {
+        if (catalogEvents == null) {
+            throw new IllegalArgumentException("catalogEvents config must not be null");
+        }
+        if (orderEvents == null) {
+            throw new IllegalArgumentException("orderEvents config must not be null");
+        }
+        if (couponIssueRequests == null) {
+            throw new IllegalArgumentException("couponIssueRequests config must not be null");
+        }
+        if (userActivityEvents == null) {
+            throw new IllegalArgumentException("userActivityEvents config must not be null");
+        }
+        if (pipelineDlq == null) {
+            throw new IllegalArgumentException("pipelineDlq config must not be null");
+        }
+    }
+
     public record TopicConfig(

설정 누락 시나리오를 커버하는 단위 테스트 추가를 권장한다.

🤖 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/config/TopicProperties.java`
around lines 15 - 21, TopicProperties 레코드에 compact constructor를 추가해
catalogEvents, orderEvents, couponIssueRequests, userActivityEvents, pipelineDlq
필드를 null 검사하도록 하세요; 누락된 YAML 바인딩으로 null이 들어오면 즉시 IllegalArgumentException(또는
NullPointerException 대신 명확한 메시지)을 던져 KafkaTopicConfig 등에서 발생할 NPE를 방지하고 디버깅용
메시지를 포함하세요; 변경 후 설정 누락 시나리오를 검증하는 단위 테스트(TopicProperties 생성 시 null 입력 시 예외 발생)를
추가하세요.

Comment on lines +27 to +28
Long retentionMs
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

retentionMs 값이 제공될 때 유효성 검증 부재

retentionMs가 null이 아닌 경우에 대한 검증이 없다. 0 또는 음수 값이 설정되면 Kafka 토픽에서 메시지가 즉시 삭제되거나 예측 불가능한 동작이 발생할 수 있다. DLQ 토픽의 경우 장애 분석을 위한 메시지가 유실될 수 있어 운영상 치명적이다.

🛡️ retentionMs 검증 추가 제안
             if (minInsyncReplicas <= 0 || minInsyncReplicas > replicas) {
                 throw new IllegalArgumentException(
                     "min-insync-replicas must be positive and <= replicas"
                 );
             }
+            if (retentionMs != null && retentionMs <= 0) {
+                throw new IllegalArgumentException(
+                    "retention-ms must be positive when specified"
+                );
+            }
         }

retentionMs에 0 또는 음수를 설정했을 때 예외가 발생하는지 확인하는 테스트 케이스 추가를 권장한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/commerce-api/src/main/java/com/loopers/config/TopicProperties.java`
around lines 27 - 28, TopicProperties 생성자에서 전달된 retentionMs에 대한 유효성 검증이 빠져 있어 0
또는 음수값이 들어갈 경우 문제가 발생하므로, TopicProperties(...) 생성자(또는 빌더 초기화 지점)에서 retentionMs가
null이 아니면 반드시 > 0인지 검사하고 그렇지 않으면 IllegalArgumentException을 던지도록 추가하세요; 검증 로직은
TopicProperties 클래스 내부(예: validateRetentionMs 메서드 또는 생성자 시작부)로 넣고 메시지에 필드명과
유효조건을 포함하여 명확히 하며, 이 동작을 보장하는 단위테스트를 추가하여 retentionMs == 0 및 retentionMs < 0
케이스에서 예외가 발생하는지 확인하세요.

Comment on lines +95 to +104
public void markFailed(String errorMessage) {
this.status = OutboxStatus.FAILED;
this.retryCount++;
this.errorMessage = errorMessage;
}

public void markRetry() {
this.status = OutboxStatus.PENDING;
this.retryCount++;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

markRetry()가 retryCount를 미리 올려 재시도 한도가 예상보다 빨리 소진된다.

지금은 실패 시점의 markFailed()와 재시도 준비 단계의 markRetry()가 모두 retryCount를 증가시킨다. 운영 경로에서는 실패 1회당 카운트가 두 번 올라가서, 설정한 재시도 횟수보다 훨씬 빨리 재시도 대상에서 제외될 수 있다. markRetry()는 상태만 재시도 가능 상태로 되돌리고, 카운트 증가는 실제 실패가 확정되는 markFailed()만 담당하도록 분리하는 편이 맞다. 추가 테스트로 동일 이벤트를 계속 실패시키며 재시도 루프를 반복했을 때 정확히 설정한 횟수만큼만 재시도되는지 검증이 필요하다.

🤖 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/outbox/OutboxEventEntity.java`
around lines 95 - 104, markRetry() currently increments retryCount causing
double-counting alongside markFailed(); update OutboxEventEntity so markRetry()
only sets status = OutboxStatus.PENDING (and does not modify retryCount) while
markFailed(String errorMessage) remains responsible for incrementing retryCount
and setting errorMessage/status, and add/adjust unit tests for OutboxEventEntity
to simulate repeated failures to assert that retryCount increases only once per
failure and that retries stop after the configured limit.

Comment on lines +88 to +117
@Scheduled(fixedDelay = 1000)
public void publishProcessingEvents() {
List<OutboxEventEntity> processingEvents = outboxEventJpaRepository.findProcessingEvents(BATCH_SIZE);

if (processingEvents.isEmpty()) {
return;
}

log.info("[Relay Phase 2] PROCESSING 이벤트 {}건 발행 시작", processingEvents.size());

// Partition Key별 그룹핑 (순서 보장)
var groupedByPartitionKey = processingEvents.stream()
.collect(java.util.stream.Collectors.groupingBy(
OutboxEventEntity::getPartitionKey,
java.util.LinkedHashMap::new,
java.util.stream.Collectors.toList()
));

// Partition Key별 병렬 발행 (다른 Key는 병렬, 같은 Key는 순차)
groupedByPartitionKey.values().parallelStream().forEach(events -> {
for (OutboxEventEntity event : events) {
publishToKafka(event);
}
});

// 상태 업데이트 (PUBLISHED or FAILED)
transactionTemplate.executeWithoutResult(status -> {
outboxEventJpaRepository.saveAll(processingEvents);
log.info("[Relay Phase 2] 상태 업데이트 완료 {}건", processingEvents.size());
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Phase 2는 아직 멀티 인스턴스 중복 발행을 막지 못한다.

Phase 1이 커밋한 뒤 PROCESSING row는 모든 인스턴스에서 동일하게 보이는데, 여기에는 owner/lease 정보가 없어서 여러 인스턴스의 publishProcessingEvents()가 같은 outbox row를 동시에 읽고 함께 발행할 수 있다. 운영에서는 쿠폰처럼 멱등 처리되는 소비자는 버틸 수 있어도, 메트릭/로깅처럼 비멱등 소비자는 값이 두 번 반영된다. Phase 1에서 인스턴스 소유권이나 lease expiry를 함께 저장하고, Phase 2는 자기 소유 row만 읽게 하거나 별도의 PUBLISHING claim 단계를 두는 편이 안전하다. 추가 테스트로 두 개의 relay 인스턴스가 같은 PROCESSING 집합에 대해 Phase 2를 동시에 돌려도 각 outboxId가 한 번만 발행되는지 검증이 필요하다.

🤖 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/outbox/OutboxRelayService.java`
around lines 88 - 117, The publishProcessingEvents() method can read the same
PROCESSING rows on multiple instances—add a lease/owner-claim step so Phase 2
only publishes rows this instance has claimed. Implement a new transactional
claim method (e.g., claimProcessingEvents(ownerId, leaseUntil, batchSize)) that
does an UPDATE ... WHERE status=PROCESSING AND (owner IS NULL OR leaseUntil <
now) LIMIT BATCH_SIZE SET owner = :ownerId, leaseUntil = :expiry and returns the
claimed outboxIds, then fetch those OutboxEventEntity rows and have
publishProcessingEvents() call publishToKafka only for those owned rows and
later set status to PUBLISHED/FAILED inside the same/another transaction;
reference functions/classes: publishProcessingEvents(),
outboxEventJpaRepository.findProcessingEvents (replace with
claimProcessingEvents), OutboxEventEntity fields owner/leaseUntil,
publishToKafka, and transactionTemplate. Also add a concurrent integration test
that boots two relay instances and asserts each outboxId is published exactly
once.

Comment on lines +135 to +143
@Scheduled(fixedDelay = 3600000)
@Transactional
public void cleanupPublishedEvents() {
ZonedDateTime cutoff = ZonedDateTime.now().minusDays(7);
List<OutboxEventEntity> oldEvents = outboxEventJpaRepository.findPublishedBefore(cutoff);
if (!oldEvents.isEmpty()) {
outboxEventJpaRepository.deleteAll(oldEvents);
log.info("[Relay] PUBLISHED 이벤트 {}건 정리 (7일 이전)", oldEvents.size());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

청소 작업이 오래된 이벤트를 전량 적재해 배치성 장애를 만들 수 있다.

이 경로는 findPublishedBefore(cutoff)로 만료 이벤트를 모두 엔티티로 읽어온 뒤 deleteAll(oldEvents)를 수행한다. outbox 처리량이 쌓이는 운영 환경에서는 수백만 건을 한 번에 메모리로 끌어와 긴 삭제 트랜잭션과 큰 heap spike를 만들 수 있다. bulk delete 쿼리를 청크 단위로 수행하거나, ID 페이징으로 잘라 지우는 방식으로 바꾸는 편이 안전하다. 추가 테스트로 청크 크기보다 많은 published row를 넣었을 때 여러 번 나눠 정리되는지 검증이 필요하다.

🤖 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/outbox/OutboxRelayService.java`
around lines 135 - 143, The cleanupPublishedEvents method currently loads all
entities via findPublishedBefore(cutoff) and calls
outboxEventJpaRepository.deleteAll(oldEvents), which can OOM and cause huge
transactions; change it to delete in bounded batches: implement a batch-delete
loop in cleanupPublishedEvents that repeatedly queries only IDs (or a Pageable
of IDs) from outboxEventJpaRepository.findPublishedBefore(cutoff,
Pageable.ofSize(batchSize)) or a custom repository method
findIdsPublishedBefore(cutoff, limit) and then call a bulk delete like
outboxEventJpaRepository.deleteAllByIdInBatch(idList) or a JPQL/SQL delete where
id IN (:ids); commit each batch (remove the big `@Transactional` scope or use a
new transaction per batch) until no more rows remain; add an integration test
that inserts >batchSize published rows and asserts the method deletes them
across multiple batches.

Comment on lines +111 to +145
@Test
@DisplayName("재시도 대상 이벤트를 조회할 수 있다")
void find_retryable_events() {
// given
OutboxEventEntity event = outboxEventService.save("ORDER", 1L, "OrderConfirmedEvent",
Map.of("orderId", 1L), "order-events-v1", "1");
event.markFailed("timeout");
outboxEventJpaRepository.save(event);

// when
List<OutboxEventEntity> retryable = outboxEventJpaRepository.findRetryableEvents(5, 50);

// then
assertThat(retryable).hasSize(1);
assertThat(retryable.get(0).getRetryCount()).isEqualTo(1);
}

@Test
@DisplayName("maxRetry 초과한 이벤트는 재시도 대상에서 제외된다")
void exceeded_max_retry_excluded() {
// given
OutboxEventEntity event = outboxEventService.save("ORDER", 1L, "OrderConfirmedEvent",
Map.of("orderId", 1L), "order-events-v1", "1");
// 5회 실패
for (int i = 0; i < 5; i++) {
event.markFailed("fail " + i);
}
outboxEventJpaRepository.save(event);

// when
List<OutboxEventEntity> retryable = outboxEventJpaRepository.findRetryableEvents(5, 50);

// then
assertThat(retryable).isEmpty();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

재시도 한도는 실제 상태 전이까지 묶어서 검증하는 편이 안전하다.

현재 테스트는 markFailed() 이후 조회 조건만 보아서 운영 경로인 FAILED -> markRetry() -> 재발행 실패 사이클을 놓치고 있다. 이 구간이 빠지면 retryCount가 두 번 올라가는 유형의 버그가 그대로 통과한다. OutboxRelayService.retryFailedEvents()와 같은 상태 전이를 반복하는 테스트를 추가해, 설정한 재시도 한도와 실제 재시도 횟수가 일치하는지 검증하는 편이 안전하다. 추가 테스트로 동일 이벤트를 연속 실패시킨 뒤 재시도 대상에서 제외되는 정확한 시점을 단정해 달라. As per coding guidelines "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."

🤖 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/outbox/OutboxEventServiceTest.java`
around lines 111 - 145, Add an integration-style unit test that exercises the
full FAILED -> markRetry() -> re-publish failure cycle instead of only calling
markFailed() and then querying findRetryableEvents; create an OutboxEventEntity
via outboxEventService.save, then call the OutboxRelayService.retryFailedEvents
(or replicate its state transitions by invoking the same
markRetry()/publish-failure path) repeatedly to simulate consecutive publish
failures, saving the entity each step, and assert that
findRetryableEvents(maxRetry, batchSize) includes the event up until the
configured maxRetry and excludes it immediately after the final failed retry;
reference OutboxEventEntity.markFailed, OutboxEventEntity.markRetry (or the
retry path inside OutboxRelayService.retryFailedEvents),
outboxEventJpaRepository.findRetryableEvents and outboxEventService.save to
locate the code to extend.

Comment on lines +230 to +231
// 각 스레드가 최소 1개 이상은 획득했는지 확인 (공정성)
assertThat(acquiredCountByThread.values()).allMatch(count -> count > 0);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

공정성 단정은 FOR UPDATE SKIP LOCKED의 보장 범위 밖이라 플래키하다.

이 쿼리는 중복 방지용이지 스레드별 공정 분배를 보장하지 않는다. 스케줄링과 커넥션 타이밍에 따라 한 스레드가 0건을 가져가도 구현은 정상일 수 있어서, 이 단정은 CI에서 간헐 실패를 만들 수 있다. 여기서는 총 획득 수와 중복 없음만 검증하고, 공정성이 실제 요구사항이면 별도의 shard/lease 정책을 코드에 넣은 뒤 그 정책을 테스트하는 편이 안전하다. 추가 테스트로 공정성 단정을 제거한 상태에서 여러 번 반복 실행해도 총합과 중복 조건만 안정적으로 유지되는지 검증이 필요하다. As per coding guidelines "통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다."

🤖 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/outbox/OutboxRelayServiceIntegrationTest.java`
around lines 230 - 231, Remove the flaky fairness assertion in
OutboxRelayServiceIntegrationTest that checks
acquiredCountByThread.values().allMatch(count -> count > 0); instead assert only
that the total number of acquired messages equals the expected total and that
there are no duplicate message IDs across threads (use acquiredCountByThread and
the collection of acquiredIds to verify uniqueness and sum), and optionally wrap
the test body in a short loop to repeat the scenario multiple times to ensure
stability; if fairness is required later, implement a deterministic shard/lease
policy in the production code and add dedicated tests for that policy rather
than asserting fairness here.

Comment on lines +75 to +77
// Spring Kafka ErrorHandler 추가 — 재시도 없이 즉시 DLQ 격리
// 배치 블로킹 방지: 한 레코드 실패 시 나머지 레코드 블로킹 없이 즉시 DLQ 전송
factory.setCommonErrorHandler(createErrorHandler(kafkaTemplate));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Spring Kafka DefaultErrorHandler batch listener BatchListenerFailedException failed record isolation official documentation

💡 Result:

In Spring Kafka batch listeners using DefaultErrorHandler, failed record isolation requires the listener to throw a BatchListenerFailedException specifying the index of the failed record. When BatchListenerFailedException is thrown: - Offsets of records before the failed index are committed. - If retries remain, seeks are performed to redeliver remaining records (including the failed one). - After retries exhausted, the failed record is recovered (default: logged at ERROR), and seeks performed for records after it. Other exceptions trigger fallback handling (whole batch retry). Official documentation: https://docs.spring.io/spring-kafka/reference/kafka/annotation-error-handling.html and DefaultErrorHandler/FailedBatchProcessor JavaDocs.

Citations:


🏁 Script executed:

# KafkaConfig의 DefaultErrorHandler 생성 방식 확인
cat -n modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java | sed -n '50,120p'

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 3243


🏁 Script executed:

# CouponIssueConsumer의 배치 리스너 구현 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java | head -80

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 4112


🏁 Script executed:

# FixedBackOff 재시도 횟수 확인
rg "FixedBackOff" modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java -A 2 -B 2

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 458


배치 리스너에서 현재 설정으로는 실패 레코드 단위 DLQ 격리가 불가능하다.

CouponIssueConsumer.consume()에서 일반 Exception을 throw하면 Spring Kafka의 DefaultErrorHandler는 실패 레코드의 index 정보를 받지 못한다. 공식 문서에 따르면 index 정보 없이 예외가 발생하면 whole batch 단위로 recovery가 실행되어 이미 성공 처리된 레코드까지 함께 DLQ로 전송된다. FixedBackOff(0L, 0L)은 재시도 횟수만 0으로 설정할 뿐 recovery 자체는 여전히 발동되므로, 배치 내 1, 3번째 레코드가 성공해도 2번째 실패 시 전체 배치가 DLQ 대상이 되어 중복 처리와 처리 지연이 발생한다.

throw new BatchListenerFailedException("...", recordIndex)로 실패 index를 명시하거나, record 단위 listener로 전환하여 DeadLetterPublishingRecoverer가 단일 레코드만 격리하도록 수정이 필요하다. 추가 검증으로 배치 크기 3에서 2번째 레코드만 실패시킬 때 1, 3번째가 실제로 DLQ로 전송되지 않는지 반드시 확인하고, 운영 배포 전에 end-to-end 테스트를 실행해야 한다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java` around
lines 75 - 77, The batch listener currently causes whole-batch DLQ recovery
because CouponIssueConsumer.consume() throws a generic Exception and
DefaultErrorHandler lacks record index; update error handling so failures
include the failed record index (throw new BatchListenerFailedException("...",
recordIndex) from CouponIssueConsumer.consume()) or convert the listener to a
record-level listener and configure the factory's error handler (where
factory.setCommonErrorHandler(createErrorHandler(kafkaTemplate)) is set) to use
DeadLetterPublishingRecoverer so only the single failed record is sent to DLQ;
after changes, add an end-to-end test that simulates batch size 3 with only the
2nd record failing and assert that records 1 and 3 are not sent to DLQ.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🧹 Nitpick comments (3)
apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxMetricsTest.java (1)

11-12: 사용되지 않는 import가 존재한다.

ZonedDateTimeOptional이 import되어 있으나 테스트 코드에서 사용되지 않는다. 향후 oldest pending age gauge 테스트 추가 시 필요할 수 있으나, 현재는 불필요하다.

🤖 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/outbox/OutboxMetricsTest.java`
around lines 11 - 12, Remove the unused imports ZonedDateTime and Optional from
the OutboxMetricsTest file; locate the import statements for ZonedDateTime and
Optional at the top of the OutboxMetricsTest class and delete them (or let your
IDE/formatter optimize imports) so the test compilation and linter warnings are
resolved.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java (1)

70-90: Gauge 메트릭이 매 스크래핑마다 DB 쿼리를 실행해 성능 병목이 될 수 있다.

Prometheus 스크래핑 주기(기본 15초)마다 countByStatus() 4회 + findOldestPendingEvent() 1회가 실행된다. 운영 환경에서 outbox 테이블이 커지면 스크래핑 타임아웃이나 DB 부하 증가로 이어질 수 있다.

개선 방안:

  1. 캐싱 레이어 추가: 짧은 TTL(예: 10초)로 조회 결과를 캐싱
  2. 또는 relay 배치 처리 시점에 카운트를 갱신하는 push 방식으로 전환

추가 테스트: 부하 상태에서 /actuator/prometheus 응답 시간 측정 필요.

♻️ 캐싱을 적용한 개선안
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.ConcurrentHashMap;
+
 `@Component`
 public class OutboxMetrics {
 
     private final MeterRegistry registry;
     private final OutboxEventJpaRepository repository;
+    
+    // 캐시된 카운트 (10초 TTL)
+    private final ConcurrentHashMap<OutboxStatus, Long> cachedCounts = new ConcurrentHashMap<>();
+    private volatile long lastCacheRefresh = 0;
+    private static final long CACHE_TTL_MS = 10_000;
 
     // ... 생략 ...
 
+    private long getCachedCount(OutboxStatus status) {
+        long now = System.currentTimeMillis();
+        if (now - lastCacheRefresh > CACHE_TTL_MS) {
+            refreshCache();
+        }
+        return cachedCounts.getOrDefault(status, 0L);
+    }
+
+    private synchronized void refreshCache() {
+        long now = System.currentTimeMillis();
+        if (now - lastCacheRefresh > CACHE_TTL_MS) {
+            for (OutboxStatus status : OutboxStatus.values()) {
+                cachedCounts.put(status, repository.countByStatus(status));
+            }
+            lastCacheRefresh = now;
+        }
+    }
🤖 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/outbox/OutboxMetrics.java`
around lines 70 - 90, The registerGauges method in OutboxMetrics registers
Prometheus gauges that call repository.countByStatus(...) and
calculateOldestPendingAge() on every scrape, which can overload the DB; change
this to use a short-lived cached value or push-updated counters: implement a
cached layer inside OutboxMetrics (e.g., use a ConcurrentHashMap or Caffeine
with ~10s TTL) to store results for each OutboxStatus and the oldest pending
age, update those cached values on a scheduled task or when the relay batch
runs, and have registry.gauge callbacks read from the cache instead of calling
repository.countByStatus(...) or calculateOldestPendingAge() directly;
alternatively, replace gauge registration with methods that increment/decrement
push counters during batch processing so no DB queries run on scrape.
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java (1)

94-109: create() 정적 메서드와 @PrePersist에서 타임스탬프를 중복 설정한다.

create()에서 createdAt/updatedAt을 설정하고, @PrePersistonCreate()에서도 동일 필드를 설정한다. 현재는 onCreate()createdAt == null 체크를 하므로 동작에 문제는 없으나, 의도가 불명확하고 향후 유지보수 시 혼란을 야기할 수 있다.

권장: create()에서 타임스탬프 설정을 제거하고 @PrePersist에 위임.

♻️ 타임스탬프 설정 단일화
     public static OutboxEventEntity create(String aggregateType, Long aggregateId,
                                             String eventType, String payload,
                                             String topic, String partitionKey) {
         OutboxEventEntity entity = new OutboxEventEntity();
         entity.aggregateType = aggregateType;
         entity.aggregateId = aggregateId;
         entity.eventType = eventType;
         entity.payload = payload;
         entity.topic = topic;
         entity.partitionKey = partitionKey;
         entity.status = OutboxStatus.PENDING;
         entity.retryCount = 0;
-        entity.createdAt = ZonedDateTime.now();
-        entity.updatedAt = ZonedDateTime.now();
+        // createdAt, updatedAt은 `@PrePersist에서` 설정됨
         return entity;
     }
🤖 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/outbox/OutboxEventEntity.java`
around lines 94 - 109, The create() factory sets createdAt/updatedAt even though
onCreate() (annotated `@PrePersist`) is responsible for initializing timestamps,
causing duplication and potential confusion; remove the createdAt and updatedAt
assignments from OutboxEventEntity.create(...) and let the `@PrePersist`
onCreate() method set those fields (keep other initializations like status and
retryCount in create()), ensuring timestamps are centralized in onCreate().
🤖 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/outbox/OutboxEventEntity.java`:
- Around line 24-33: The JPA `@Index` annotations on the OutboxEventEntity
(`@Table`(..., indexes = {...})) conflict with Flyway-created partial indexes;
remove the `@Index` entries from the `@Table` annotation in class OutboxEventEntity
so JPA won't attempt to create non-partial indexes at runtime, and replace them
with a clear inline comment above the entity noting that partial indexes are
created and managed by Flyway (include which partial indexes: pending by
created_at, processing by created_at/updated_at and status indices). Also update
any related documentation/CI checks to assert via EXPLAIN that the DB actually
has the partial indexes created by Flyway.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java`:
- Around line 97-104: calculateOldestPendingAge uses ZonedDateTime.now() without
specifying timezone which can mismatch event.getCreatedAt()’s zone; update
calculateOldestPendingAge (and the repository.findOldestPendingEvent mapping) to
compute age using the same timezone as event.getCreatedAt() (e.g., call now(...
) with event.getCreatedAt().getZone() or convert both to Instant and compute
Duration between instants) so the Duration.between uses matching time
references.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java`:
- Around line 283-295: recoverProcessingOnShutdown() loads all PROCESSING events
with Integer.MAX_VALUE which can OOM; change it to process in bounded batches or
delegate to stalled recovery: remove the unbounded
findProcessingEvents(Integer.MAX_VALUE) call and instead fetch limited pages
(e.g., using outboxEventJpaRepository.findProcessingEvents(Pageable) or a
findProcessingEvents(limit, offset) loop) and for each batch call
event.markRetry() and outboxEventJpaRepository.saveAll(batch) until no more
results, or alternatively log the count and return early delegating to
recoverStalledProcessingEvents(); adjust transaction scope so each batch commits
(i.e., make recoverProcessingOnShutdown non-transactional or perform per-batch
transactions) and replace Integer.MAX_VALUE with a constant BATCH_SIZE.
- Around line 138-150: The current use of
groupedByPartitionKey.values().parallelStream() in OutboxRelayService can starve
the ForkJoinPool.commonPool; change to use a dedicated ExecutorService (e.g., a
fixed-size relayExecutor created on service init) and submit each
partition-group processing task to that executor instead of parallelStream so
you control parallelism; in the task for each group (the Runnable that iterates
events and calls publishToKafka) keep the existing per-event try/catch,
metrics.recordPublishFailure(), and event.markFailed(...) logic, and ensure
relayExecutor is properly shutdown on service stop and errors are
propagated/awaited as needed to avoid leaks.

In
`@apps/commerce-api/src/main/resources/db/migration/V1__Add_updated_at_and_indexes.sql`:
- Around line 7-10: The UPDATE on outbox_event won't affect existing rows
because updated_at was added as NOT NULL DEFAULT CURRENT_TIMESTAMP(6), so no
rows will be NULL; change the migration to either (a) add updated_at as nullable
(no DEFAULT) on outbox_event, run the UPDATE to set updated_at = created_at,
then alter the column to NOT NULL DEFAULT CURRENT_TIMESTAMP(6), or (b) keep the
current column definition but remove the WHERE updated_at IS NULL from the
UPDATE so all rows are assigned created_at; reference the outbox_event table and
the updated_at/created_at column names when making the change.
- Around line 14-23: Replace the PostgreSQL partial-index statements on
outbox_event with MySQL-compatible composite indexes: update the definitions for
idx_outbox_processing_updated, idx_outbox_pending_created, and
idx_outbox_processing_created to include status as the first column (e.g., index
on (status, updated_at) and (status, created_at) respectively) so MySQL can
create them; after editing the CREATE INDEX statements for those three indexes,
run the migration locally (e.g., flyway:migrate) to confirm there are no syntax
errors.
- Around line 25-26: The comments claiming existing composite indexes "(status,
created_at)" and "(status, updated_at)" are inaccurate; update the migration
comments to reflect the actual indexes being created (partial indexes) by either
removing the misleading lines or replacing them with a brief note describing the
partial-index strategy and the actual index names: idx_outbox_processing_updated
(on updated_at, partial WHERE status = ...), idx_outbox_pending_created (on
created_at, partial WHERE status = ...), and idx_outbox_processing_created (on
created_at, partial WHERE status = ...); ensure the comment clearly states these
are partial indexes rather than composite (status, created_at)/(status,
updated_at).

In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxGracefulShutdownTest.java`:
- Around line 70-82: The test currently only calls
relayService.recoverStalledProcessingEvents() without simulating a true
"stalled" event; to fix it, create real PROCESSING events, mark them as
PROCESSING (use OutboxEventEntity.markProcessing() via repository.saveAll or
outboxEventService), then force their updated_at to >5 minutes in the past
(perform a native UPDATE through EntityManager or add a repository helper to set
updated_at) before calling relayService.recoverStalledProcessingEvents();
finally assert that repository.countByStatus(OutboxStatus.PROCESSING) == 0 and
repository.countByStatus(OutboxStatus.PENDING) == <expected>, or alternatively
make the staleness threshold injectable in the class under test (e.g., pass a
threshold to recoverStalledProcessingEvents or set a bean property) and call it
with a short threshold so the test can simulate recovery without native SQL.
- Around line 38-41: OutboxGracefulShutdownTest leaves
OutboxRelayService.shuttingDown set after certain tests, causing cross-test
interference; fix by resetting that state between tests—either instantiate a
fresh OutboxRelayService in `@BeforeEach` for the test class or add code in
`@AfterEach` to set OutboxRelayService.shuttingDown = false (or call a provided
reset method on OutboxRelayService) so each test starts with shuttingDown
cleared.
- Around line 99-108: The test's comment about "PROCESSING 그대로 유지 (Phase 2 스킵됨)"
is inconsistent with the actual setup: calling relayService.onShutdown()
triggers recoverProcessingOnShutdown() and converts PROCESSING to PENDING, so
update the test to match the intended "Phase 2 skip" behavior by either (A)
change the comment to reflect that onShutdown() already restores PROCESSING to
PENDING, or (B) modify the setup to simulate shuttingDown without invoking
relayService.onShutdown() — e.g., set the shuttingDown flag/state directly (so
relayService.publishProcessingEvents() is skipped) and then assert
repository.countByStatus(OutboxStatus.PROCESSING) remains as expected while
PENDING count is unchanged; reference relayService.onShutdown(),
relayService.publishProcessingEvents(), recoverProcessingOnShutdown(), and
repository.countByStatus(OutboxStatus.PROCESSING)/OutboxStatus.PENDING to locate
the code to change.

In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxMetricsTest.java`:
- Around line 83-101: The tests in OutboxMetricsTest only assert the presence of
gauges but not their reported values; update the pendingCountGaugeIsRegistered
and processingCountGaugeIsRegistered tests to also assert the gauge value
matches the stubbed repository.countByStatus result (e.g., after
when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L) assert that
registry.find("outbox.pending.count").gauge().value() equals 5.0, and likewise
for OutboxStatus.PROCESSING with "outbox.processing.count" and the expected
numeric value) so the gauge wiring and value propagation from
repository.countByStatus are verified.

---

Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java`:
- Around line 94-109: The create() factory sets createdAt/updatedAt even though
onCreate() (annotated `@PrePersist`) is responsible for initializing timestamps,
causing duplication and potential confusion; remove the createdAt and updatedAt
assignments from OutboxEventEntity.create(...) and let the `@PrePersist`
onCreate() method set those fields (keep other initializations like status and
retryCount in create()), ensuring timestamps are centralized in onCreate().

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java`:
- Around line 70-90: The registerGauges method in OutboxMetrics registers
Prometheus gauges that call repository.countByStatus(...) and
calculateOldestPendingAge() on every scrape, which can overload the DB; change
this to use a short-lived cached value or push-updated counters: implement a
cached layer inside OutboxMetrics (e.g., use a ConcurrentHashMap or Caffeine
with ~10s TTL) to store results for each OutboxStatus and the oldest pending
age, update those cached values on a scheduled task or when the relay batch
runs, and have registry.gauge callbacks read from the cache instead of calling
repository.countByStatus(...) or calculateOldestPendingAge() directly;
alternatively, replace gauge registration with methods that increment/decrement
push counters during batch processing so no DB queries run on scrape.

In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxMetricsTest.java`:
- Around line 11-12: Remove the unused imports ZonedDateTime and Optional from
the OutboxMetricsTest file; locate the import statements for ZonedDateTime and
Optional at the top of the OutboxMetricsTest class and delete them (or let your
IDE/formatter optimize imports) so the test compilation and linter warnings are
resolved.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b1c0ded7-2386-4919-b4bd-9e1acacfc030

📥 Commits

Reviewing files that changed from the base of the PR and between e0230cc and f1c277b.

📒 Files selected for processing (7)
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventEntity.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayService.java
  • apps/commerce-api/src/main/resources/db/migration/V1__Add_updated_at_and_indexes.sql
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxGracefulShutdownTest.java
  • apps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxMetricsTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java

Comment on lines +24 to +33
@Table(name = "outbox_event", indexes = {
@Index(name = "idx_outbox_status_created", columnList = "status, created_at"),
@Index(name = "idx_outbox_status_updated", columnList = "status, updated_at"),
// PENDING 조회 최적화 (Phase 1)
@Index(name = "idx_outbox_pending_created", columnList = "created_at"),
// PROCESSING 조회 최적화 (Phase 2)
@Index(name = "idx_outbox_processing_created", columnList = "created_at"),
// PROCESSING 복구 최적화 (updated_at 기준)
@Index(name = "idx_outbox_processing_updated", columnList = "updated_at")
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

JPA @Index 어노테이션과 Flyway 마이그레이션이 인덱스를 중복 정의하고 있다.

JPA @Index는 partial index(WHERE 조건)를 지원하지 않으므로, Flyway에서 생성한 partial index와 이름은 같지만 정의가 다른 일반 인덱스를 생성하려 시도한다. ddl-auto=validate 모드에서는 스키마 불일치 오류가, ddl-auto=update 모드에서는 예기치 않은 인덱스 변경이 발생할 수 있다.

운영 권장:

  • ddl-auto=validate 또는 none 사용 시: JPA @Index 어노테이션 제거, Flyway만으로 인덱스 관리
  • JPA @Index는 문서화 목적으로만 유지하되 주석으로 partial index임을 명시

추가 테스트: CI에서 EXPLAIN 검증으로 실제 생성된 인덱스가 partial index인지 확인 필요.

🛡️ JPA `@Index` 제거 및 주석으로 문서화하는 수정안
 `@Entity`
-@Table(name = "outbox_event", indexes = {
-        `@Index`(name = "idx_outbox_status_created", columnList = "status, created_at"),
-        `@Index`(name = "idx_outbox_status_updated", columnList = "status, updated_at"),
-        // PENDING 조회 최적화 (Phase 1)
-        `@Index`(name = "idx_outbox_pending_created", columnList = "created_at"),
-        // PROCESSING 조회 최적화 (Phase 2)
-        `@Index`(name = "idx_outbox_processing_created", columnList = "created_at"),
-        // PROCESSING 복구 최적화 (updated_at 기준)
-        `@Index`(name = "idx_outbox_processing_updated", columnList = "updated_at")
-})
+@Table(name = "outbox_event")
+/*
+ * 인덱스는 Flyway 마이그레이션(V1__Add_updated_at_and_indexes.sql)에서 관리
+ * - idx_outbox_status_created: (status, created_at)
+ * - idx_outbox_status_updated: (status, updated_at)
+ * - idx_outbox_pending_created: PARTIAL INDEX on created_at WHERE status='PENDING'
+ * - idx_outbox_processing_created: PARTIAL INDEX on created_at WHERE status='PROCESSING'
+ * - idx_outbox_processing_updated: PARTIAL INDEX on updated_at WHERE status='PROCESSING'
+ */
 public class OutboxEventEntity {
🤖 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/outbox/OutboxEventEntity.java`
around lines 24 - 33, The JPA `@Index` annotations on the OutboxEventEntity
(`@Table`(..., indexes = {...})) conflict with Flyway-created partial indexes;
remove the `@Index` entries from the `@Table` annotation in class OutboxEventEntity
so JPA won't attempt to create non-partial indexes at runtime, and replace them
with a clear inline comment above the entity noting that partial indexes are
created and managed by Flyway (include which partial indexes: pending by
created_at, processing by created_at/updated_at and status indices). Also update
any related documentation/CI checks to assert via EXPLAIN that the DB actually
has the partial indexes created by Flyway.

Comment on lines +97 to +104
private double calculateOldestPendingAge() {
return repository.findOldestPendingEvent()
.map(event -> {
Duration age = Duration.between(event.getCreatedAt(), ZonedDateTime.now());
return (double) age.getSeconds();
})
.orElse(0.0);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

ZonedDateTime.now() 사용 시 타임존 명시가 필요하다.

서버 기본 타임존과 DB/엔티티의 타임존이 다르면 age 계산이 부정확해질 수 있다. 엔티티의 createdAt과 동일한 타임존을 명시적으로 사용해야 한다.

🛡️ 타임존 명시 수정안
     private double calculateOldestPendingAge() {
         return repository.findOldestPendingEvent()
                 .map(event -> {
-                    Duration age = Duration.between(event.getCreatedAt(), ZonedDateTime.now());
+                    Duration age = Duration.between(event.getCreatedAt(), ZonedDateTime.now(event.getCreatedAt().getZone()));
                     return (double) age.getSeconds();
                 })
                 .orElse(0.0);
     }
📝 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
private double calculateOldestPendingAge() {
return repository.findOldestPendingEvent()
.map(event -> {
Duration age = Duration.between(event.getCreatedAt(), ZonedDateTime.now());
return (double) age.getSeconds();
})
.orElse(0.0);
}
private double calculateOldestPendingAge() {
return repository.findOldestPendingEvent()
.map(event -> {
Duration age = Duration.between(event.getCreatedAt(), ZonedDateTime.now(event.getCreatedAt().getZone()));
return (double) age.getSeconds();
})
.orElse(0.0);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxMetrics.java`
around lines 97 - 104, calculateOldestPendingAge uses ZonedDateTime.now()
without specifying timezone which can mismatch event.getCreatedAt()’s zone;
update calculateOldestPendingAge (and the repository.findOldestPendingEvent
mapping) to compute age using the same timezone as event.getCreatedAt() (e.g.,
call now(... ) with event.getCreatedAt().getZone() or convert both to Instant
and compute Duration between instants) so the Duration.between uses matching
time references.

Comment on lines +138 to +150
groupedByPartitionKey.values().parallelStream().forEach(events -> {
for (OutboxEventEntity event : events) {
try {
publishToKafka(event);
} catch (Exception e) {
// 개별 이벤트 실패해도 다른 이벤트 계속 처리
log.error("[Relay Phase 2] Unexpected error processing event {}: {}",
event.getId(), e.getMessage(), e);
event.markFailed("Unexpected error: " + e.getMessage());
metrics.recordPublishFailure();
}
}
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

parallelStream()이 공용 ForkJoinPool을 사용해 다른 병렬 작업에 영향을 줄 수 있다.

parallelStream()은 기본적으로 ForkJoinPool.commonPool()을 사용한다. Kafka 전송이 지연되면 동일 JVM 내 다른 parallelStream 작업도 스레드 부족으로 지연된다. 특히 outbox 배치 크기가 500건이고 partition key가 많으면 common pool이 오래 점유된다.

권장: 전용 ExecutorService를 사용하거나, partition key 그룹 수 제한을 두어 병렬도를 제어해야 한다.

♻️ 전용 ExecutorService 사용 수정안
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
 `@Service`
 public class OutboxRelayService {
 
+    private static final int PARALLEL_THREADS = 4;
+    private final ExecutorService phase2Executor = Executors.newFixedThreadPool(PARALLEL_THREADS);
+
     // ... 생략 ...
 
     private void executePhase2() {
         // ... 생략 ...
 
-        // Partition Key별 병렬 발행 (다른 Key는 병렬, 같은 Key는 순차)
-        groupedByPartitionKey.values().parallelStream().forEach(events -> {
+        // Partition Key별 병렬 발행 (전용 스레드풀 사용)
+        var futures = groupedByPartitionKey.values().stream()
+            .map(events -> java.util.concurrent.CompletableFuture.runAsync(() -> {
+                for (OutboxEventEntity event : events) {
+                    try {
+                        publishToKafka(event);
+                    } catch (Exception e) {
+                        log.error("[Relay Phase 2] Unexpected error: {}", e.getMessage(), e);
+                        event.markFailed("Unexpected error: " + e.getMessage());
+                        metrics.recordPublishFailure();
+                    }
+                }
+            }, phase2Executor))
+            .toList();
+
+        // 모든 partition 처리 완료 대기
+        futures.forEach(f -> {
+            try {
+                f.join();
+            } catch (Exception e) {
+                log.error("[Relay Phase 2] Future join error", e);
+            }
+        });

추가 테스트: 높은 partition key 다양성 환경에서 Phase 2 처리 시간과 common pool 점유율 측정 필요.

🤖 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/outbox/OutboxRelayService.java`
around lines 138 - 150, The current use of
groupedByPartitionKey.values().parallelStream() in OutboxRelayService can starve
the ForkJoinPool.commonPool; change to use a dedicated ExecutorService (e.g., a
fixed-size relayExecutor created on service init) and submit each
partition-group processing task to that executor instead of parallelStream so
you control parallelism; in the task for each group (the Runnable that iterates
events and calls publishToKafka) keep the existing per-event try/catch,
metrics.recordPublishFailure(), and event.markFailed(...) logic, and ensure
relayExecutor is properly shutdown on service stop and errors are
propagated/awaited as needed to avoid leaks.

Comment on lines +283 to +295
@Transactional
protected void recoverProcessingOnShutdown() {
List<OutboxEventEntity> processingEvents = outboxEventJpaRepository
.findProcessingEvents(Integer.MAX_VALUE);

if (!processingEvents.isEmpty()) {
log.info("[Relay] Recovering {} PROCESSING events to PENDING on shutdown",
processingEvents.size());

processingEvents.forEach(event -> event.markRetry());
outboxEventJpaRepository.saveAll(processingEvents);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

recoverProcessingOnShutdown()Integer.MAX_VALUE로 조회하여 대량 데이터 시 OOM이 발생할 수 있다.

shutdown 시점에 PROCESSING 상태 이벤트가 다량 존재하면 메모리 부족으로 복구가 실패할 수 있다. 특히 비정상 종료 후 재시작 시 이 경로가 호출되지 않으므로, 운영 환경에서는 recoverStalledProcessingEvents()에 의존해야 한다.

권장: 배치 단위로 복구하거나, shutdown 시에는 로그만 남기고 stalled recovery에 위임하는 방식 검토.

♻️ 배치 단위 복구 수정안
     `@Transactional`
     protected void recoverProcessingOnShutdown() {
-        List<OutboxEventEntity> processingEvents = outboxEventJpaRepository
-                .findProcessingEvents(Integer.MAX_VALUE);
+        int recovered = 0;
+        List<OutboxEventEntity> batch;
+        do {
+            batch = outboxEventJpaRepository.findProcessingEvents(BATCH_SIZE);
+            if (!batch.isEmpty()) {
+                batch.forEach(OutboxEventEntity::markRetry);
+                outboxEventJpaRepository.saveAll(batch);
+                recovered += batch.size();
+            }
+        } while (!batch.isEmpty());
 
-        if (!processingEvents.isEmpty()) {
-            log.info("[Relay] Recovering {} PROCESSING events to PENDING on shutdown",
-                    processingEvents.size());
-
-            processingEvents.forEach(event -> event.markRetry());
-            outboxEventJpaRepository.saveAll(processingEvents);
+        if (recovered > 0) {
+            log.info("[Relay] Recovered {} PROCESSING events to PENDING on shutdown", recovered);
         }
     }
🤖 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/outbox/OutboxRelayService.java`
around lines 283 - 295, recoverProcessingOnShutdown() loads all PROCESSING
events with Integer.MAX_VALUE which can OOM; change it to process in bounded
batches or delegate to stalled recovery: remove the unbounded
findProcessingEvents(Integer.MAX_VALUE) call and instead fetch limited pages
(e.g., using outboxEventJpaRepository.findProcessingEvents(Pageable) or a
findProcessingEvents(limit, offset) loop) and for each batch call
event.markRetry() and outboxEventJpaRepository.saveAll(batch) until no more
results, or alternatively log the count and return early delegating to
recoverStalledProcessingEvents(); adjust transaction scope so each batch commits
(i.e., make recoverProcessingOnShutdown non-transactional or perform per-batch
transactions) and replace Integer.MAX_VALUE with a constant BATCH_SIZE.

Comment on lines +7 to +10
-- 기존 데이터 updated_at 초기화 (created_at과 동일)
UPDATE outbox_event
SET updated_at = created_at
WHERE updated_at IS NULL;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

UPDATE ... WHERE updated_at IS NULL 문이 실행되지 않을 가능성이 높다.

Line 5에서 NOT NULL DEFAULT CURRENT_TIMESTAMP(6)으로 컬럼을 추가하면, 기존 row에도 default 값이 적용된다. 따라서 WHERE updated_at IS NULL 조건을 만족하는 row가 없어 UPDATE가 0건 처리된다.

의도가 created_at과 동일하게 초기화하는 것이라면:

  1. DEFAULT 없이 nullable로 컬럼 추가 → UPDATE → NOT NULL 제약 추가 순서로 변경
  2. 또는 무조건 UPDATE 실행 (WHERE 절 제거)
🛡️ 의도대로 동작하는 수정안
 -- updated_at 컬럼 추가
 ALTER TABLE outbox_event
-ADD COLUMN updated_at TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6);
+ADD COLUMN updated_at TIMESTAMP(6);

 -- 기존 데이터 updated_at 초기화 (created_at과 동일)
 UPDATE outbox_event
-SET updated_at = created_at
-WHERE updated_at IS NULL;
+SET updated_at = created_at;
+
+-- NOT NULL 제약 추가
+ALTER TABLE outbox_event
+ALTER COLUMN updated_at SET NOT NULL,
+ALTER COLUMN updated_at SET DEFAULT CURRENT_TIMESTAMP(6);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/resources/db/migration/V1__Add_updated_at_and_indexes.sql`
around lines 7 - 10, The UPDATE on outbox_event won't affect existing rows
because updated_at was added as NOT NULL DEFAULT CURRENT_TIMESTAMP(6), so no
rows will be NULL; change the migration to either (a) add updated_at as nullable
(no DEFAULT) on outbox_event, run the UPDATE to set updated_at = created_at,
then alter the column to NOT NULL DEFAULT CURRENT_TIMESTAMP(6), or (b) keep the
current column definition but remove the WHERE updated_at IS NULL from the
UPDATE so all rows are assigned created_at; reference the outbox_event table and
the updated_at/created_at column names when making the change.

Comment on lines +38 to +41
@AfterEach
void tearDown() {
databaseCleanUp.truncateAllTables();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

테스트 간 상태 격리가 불완전할 수 있다.

databaseCleanUp.truncateAllTables()로 DB 상태는 정리되지만, OutboxRelayServiceshuttingDown 플래그는 초기화되지 않는다. gracefulShutdownRecoversPROCESSINGtoPENDING() 또는 phase2SkipsWhenShuttingDown() 이후 다른 테스트가 실행되면 shuttingDown=true 상태가 유지되어 예기치 않은 동작이 발생할 수 있다.

권장: @BeforeEach 또는 @AfterEach에서 shuttingDown 플래그를 초기화하거나, 테스트마다 새 OutboxRelayService 인스턴스를 사용해야 한다.

🛡️ 상태 초기화 추가안
+import org.springframework.test.util.ReflectionTestUtils;
+import java.util.concurrent.atomic.AtomicBoolean;
+
     `@AfterEach`
     void tearDown() {
         databaseCleanUp.truncateAllTables();
+        // OutboxRelayService의 shuttingDown 플래그 초기화
+        ReflectionTestUtils.setField(relayService, "shuttingDown", new AtomicBoolean(false));
     }
🤖 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/outbox/OutboxGracefulShutdownTest.java`
around lines 38 - 41, OutboxGracefulShutdownTest leaves
OutboxRelayService.shuttingDown set after certain tests, causing cross-test
interference; fix by resetting that state between tests—either instantiate a
fresh OutboxRelayService in `@BeforeEach` for the test class or add code in
`@AfterEach` to set OutboxRelayService.shuttingDown = false (or call a provided
reset method on OutboxRelayService) so each test starts with shuttingDown
cleared.

Comment on lines +70 to +82
@Test
@DisplayName("5분 이상 PROCESSING 이벤트 복구")
void recoverStalledProcessingEvents() {
// given: PROCESSING 이벤트 생성 (5분 이상 경과 시뮬레이션 어려움)
// 실제 운영에서는 updated_at이 5분 이전인 이벤트를 찾아서 복구
// 이 테스트는 복구 메서드 호출만 검증

// when
relayService.recoverStalledProcessingEvents();

// then: 에러 없이 실행됨
assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(0);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

recoverStalledProcessingEvents() 테스트가 실제 stalled 상황을 검증하지 않는다.

테스트 주석에 "5분 이상 경과 시뮬레이션 어려움"이라고 명시되어 있으나, 이는 테스트 커버리지 gap이다. updated_at을 과거 시점으로 직접 설정하거나, threshold를 주입 가능하게 리팩터링하여 실제 복구 로직을 검증해야 한다.

💚 실제 stalled 복구를 검증하는 테스트 추가안
`@Test`
`@DisplayName`("5분 이상 PROCESSING 이벤트가 PENDING으로 복구된다")
void recoverStalledProcessingEvents_actuallyRecovers() {
    // given: PROCESSING 이벤트 생성 후 updated_at을 6분 전으로 설정
    for (int i = 1; i <= 3; i++) {
        outboxEventService.save("ORDER", (long) i, "OrderConfirmedEvent",
                Map.of("orderId", i), "order-events-v1", String.valueOf(i));
    }

    transactionTemplate.executeWithoutResult(status -> {
        List<OutboxEventEntity> pending = repository.findPendingEventsForUpdate(50);
        pending.forEach(OutboxEventEntity::markProcessing);
        repository.saveAll(pending);
    });

    // updated_at을 6분 전으로 직접 UPDATE (native query 필요)
    transactionTemplate.executeWithoutResult(status -> {
        repository.findAll().forEach(e -> {
            // EntityManager를 통한 native update 또는 별도 repository method 필요
        });
    });

    // when
    relayService.recoverStalledProcessingEvents();

    // then: PROCESSING → PENDING 복구됨
    assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(0);
    assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(3);
}
🤖 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/outbox/OutboxGracefulShutdownTest.java`
around lines 70 - 82, The test currently only calls
relayService.recoverStalledProcessingEvents() without simulating a true
"stalled" event; to fix it, create real PROCESSING events, mark them as
PROCESSING (use OutboxEventEntity.markProcessing() via repository.saveAll or
outboxEventService), then force their updated_at to >5 minutes in the past
(perform a native UPDATE through EntityManager or add a repository helper to set
updated_at) before calling relayService.recoverStalledProcessingEvents();
finally assert that repository.countByStatus(OutboxStatus.PROCESSING) == 0 and
repository.countByStatus(OutboxStatus.PENDING) == <expected>, or alternatively
make the staleness threshold injectable in the class under test (e.g., pass a
threshold to recoverStalledProcessingEvents or set a bean property) and call it
with a short threshold so the test can simulate recovery without native SQL.

Comment on lines +99 to +108
// when: Shutdown 플래그 설정
relayService.onShutdown();

// when: Phase 2 호출 시도 (shuttingDown=true이므로 스킵)
relayService.publishProcessingEvents();

// then: PROCESSING 그대로 유지 (Phase 2 스킵됨)
assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(0); // 이미 복원됨
assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(5);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

테스트 주석과 assertion이 불일치한다.

Line 105 주석은 "PROCESSING 그대로 유지 (Phase 2 스킵됨)"라고 하지만, Line 100의 onShutdown() 호출이 이미 recoverProcessingOnShutdown()을 통해 PROCESSING을 PENDING으로 복원한다. 따라서 assertion은 정확하지만 주석이 잘못되었다.

테스트 의도가 "Phase 2 스킵"을 검증하는 것이라면, onShutdown() 없이 shuttingDown 플래그만 설정하는 방식으로 수정해야 한다.

💚 테스트 의도에 맞는 수정안
     `@Test`
     `@DisplayName`("Shutdown 플래그 설정 시 Phase 2 건너뜀")
     void phase2SkipsWhenShuttingDown() {
         // given: PROCESSING 이벤트 생성
         for (int i = 1; i <= 5; i++) {
             outboxEventService.save("ORDER", (long) i, "OrderConfirmedEvent",
                     Map.of("orderId", i), "order-events-v1", String.valueOf(i));
         }

         transactionTemplate.executeWithoutResult(status -> {
             List<OutboxEventEntity> pending = repository.findPendingEventsForUpdate(50);
             pending.forEach(OutboxEventEntity::markProcessing);
             repository.saveAll(pending);
         });

-        // when: Shutdown 플래그 설정
-        relayService.onShutdown();
+        // when: Shutdown 플래그만 설정 (복구 로직 호출 없이)
+        // ReflectionTestUtils를 사용해 shuttingDown 플래그만 true로 설정
+        ReflectionTestUtils.setField(relayService, "shuttingDown", new AtomicBoolean(true));

         // when: Phase 2 호출 시도 (shuttingDown=true이므로 스킵)
         relayService.publishProcessingEvents();

-        // then: PROCESSING 그대로 유지 (Phase 2 스킵됨)
-        assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(0); // 이미 복원됨
-        assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(5);
+        // then: PROCESSING 그대로 유지 (Phase 2 스킵됨, 복구도 안 됨)
+        assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(5);
+        assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(0);
     }
📝 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
// when: Shutdown 플래그 설정
relayService.onShutdown();
// when: Phase 2 호출 시도 (shuttingDown=true이므로 스킵)
relayService.publishProcessingEvents();
// then: PROCESSING 그대로 유지 (Phase 2 스킵됨)
assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(0); // 이미 복원됨
assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(5);
}
// when: Shutdown 플래그만 설정 (복구 로직 호출 없이)
// ReflectionTestUtils를 사용해 shuttingDown 플래그만 true로 설정
ReflectionTestUtils.setField(relayService, "shuttingDown", new AtomicBoolean(true));
// when: Phase 2 호출 시도 (shuttingDown=true이므로 스킵)
relayService.publishProcessingEvents();
// then: PROCESSING 그대로 유지 (Phase 2 스킵됨, 복구도 안 됨)
assertThat(repository.countByStatus(OutboxStatus.PROCESSING)).isEqualTo(5);
assertThat(repository.countByStatus(OutboxStatus.PENDING)).isEqualTo(0);
}
🤖 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/outbox/OutboxGracefulShutdownTest.java`
around lines 99 - 108, The test's comment about "PROCESSING 그대로 유지 (Phase 2
스킵됨)" is inconsistent with the actual setup: calling relayService.onShutdown()
triggers recoverProcessingOnShutdown() and converts PROCESSING to PENDING, so
update the test to match the intended "Phase 2 skip" behavior by either (A)
change the comment to reflect that onShutdown() already restores PROCESSING to
PENDING, or (B) modify the setup to simulate shuttingDown without invoking
relayService.onShutdown() — e.g., set the shuttingDown flag/state directly (so
relayService.publishProcessingEvents() is skipped) and then assert
repository.countByStatus(OutboxStatus.PROCESSING) remains as expected while
PENDING count is unchanged; reference relayService.onShutdown(),
relayService.publishProcessingEvents(), recoverProcessingOnShutdown(), and
repository.countByStatus(OutboxStatus.PROCESSING)/OutboxStatus.PENDING to locate
the code to change.

Comment on lines +83 to +101
@Test
@DisplayName("PENDING 개수 Gauge가 등록되어 있다")
void pendingCountGaugeIsRegistered() {
// when
when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L);

// then
assertThat(registry.find("outbox.pending.count").gauge()).isNotNull();
}

@Test
@DisplayName("PROCESSING 개수 Gauge가 등록되어 있다")
void processingCountGaugeIsRegistered() {
// when
when(repository.countByStatus(OutboxStatus.PROCESSING)).thenReturn(3L);

// then
assertThat(registry.find("outbox.processing.count").gauge()).isNotNull();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Gauge 테스트가 등록 여부만 검증하고 실제 값 반영을 검증하지 않는다.

when(repository.countByStatus(...)).thenReturn(5L) stub을 설정했으나, gauge가 해당 값을 실제로 반환하는지 검증하지 않는다. Gauge 동작 검증이 미흡하면 런타임 시 DB 연결 문제나 쿼리 오류를 조기에 발견하지 못한다.

💚 Gauge 값 검증을 추가한 테스트 수정안
     `@Test`
     `@DisplayName`("PENDING 개수 Gauge가 등록되어 있다")
-    void pendingCountGaugeIsRegistered() {
+    void pendingCountGaugeReturnsRepositoryValue() {
         // when
         when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L);
 
         // then
-        assertThat(registry.find("outbox.pending.count").gauge()).isNotNull();
+        var gauge = registry.find("outbox.pending.count").gauge();
+        assertThat(gauge).isNotNull();
+        assertThat(gauge.value()).isEqualTo(5.0);
     }
 
     `@Test`
     `@DisplayName`("PROCESSING 개수 Gauge가 등록되어 있다")
-    void processingCountGaugeIsRegistered() {
+    void processingCountGaugeReturnsRepositoryValue() {
         // when
         when(repository.countByStatus(OutboxStatus.PROCESSING)).thenReturn(3L);
 
         // then
-        assertThat(registry.find("outbox.processing.count").gauge()).isNotNull();
+        var gauge = registry.find("outbox.processing.count").gauge();
+        assertThat(gauge).isNotNull();
+        assertThat(gauge.value()).isEqualTo(3.0);
     }
📝 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
@Test
@DisplayName("PENDING 개수 Gauge가 등록되어 있다")
void pendingCountGaugeIsRegistered() {
// when
when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L);
// then
assertThat(registry.find("outbox.pending.count").gauge()).isNotNull();
}
@Test
@DisplayName("PROCESSING 개수 Gauge가 등록되어 있다")
void processingCountGaugeIsRegistered() {
// when
when(repository.countByStatus(OutboxStatus.PROCESSING)).thenReturn(3L);
// then
assertThat(registry.find("outbox.processing.count").gauge()).isNotNull();
}
`@Test`
`@DisplayName`("PENDING 개수 Gauge가 등록되어 있다")
void pendingCountGaugeReturnsRepositoryValue() {
// when
when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L);
// then
var gauge = registry.find("outbox.pending.count").gauge();
assertThat(gauge).isNotNull();
assertThat(gauge.value()).isEqualTo(5.0);
}
`@Test`
`@DisplayName`("PROCESSING 개수 Gauge가 등록되어 있다")
void processingCountGaugeReturnsRepositoryValue() {
// when
when(repository.countByStatus(OutboxStatus.PROCESSING)).thenReturn(3L);
// then
var gauge = registry.find("outbox.processing.count").gauge();
assertThat(gauge).isNotNull();
assertThat(gauge.value()).isEqualTo(3.0);
}
🤖 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/outbox/OutboxMetricsTest.java`
around lines 83 - 101, The tests in OutboxMetricsTest only assert the presence
of gauges but not their reported values; update the
pendingCountGaugeIsRegistered and processingCountGaugeIsRegistered tests to also
assert the gauge value matches the stubbed repository.countByStatus result
(e.g., after when(repository.countByStatus(OutboxStatus.PENDING)).thenReturn(5L)
assert that registry.find("outbox.pending.count").gauge().value() equals 5.0,
and likewise for OutboxStatus.PROCESSING with "outbox.processing.count" and the
expected numeric value) so the gauge wiring and value propagation from
repository.countByStatus are verified.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant