Test/k6 test : 동시성 테스트 및 최종 선택

[Max-1012]

동시성 문제 테스트한 코드 및 최종선택한 Redisson Blocking 적용 버전입니다.

[github-actions]

🤖 AI 코드리뷰

리더보드 — 현재 4위 (리뷰 104건) | 전체 8개 팀 참여 중
업데이트 확인 — 리뷰가 질문을 던지다 — 생각해보기

안녕하세요! 👒 코드 리뷰를 담당하게 된 멘토입니다.

이번 PR에서 동시성 제어라는 어려운 주제에 대해 다양한 락(Lock) 전략을 구현하고, K6를 활용해 직접 성능을 비교하시는 모습이 정말 멋집니다. 실무에서도 이런 식의 체계적인 테스트 기반의 결정이 매우 중요합니다.

요구사항과 코드를 꼼꼼히 확인하고, 성장에 도움이 될 만한 피드백을 정리해 보았습니다.

---

📋 과제 요구사항 준수 여부

• 필수 기능 (동시성 제어):
  • Lettuce 기반 Redis Lock (SETNX, Spin Lock 등) 구현 및 테스트 코드 작성: 이전 PR에서 이미 구현하신 것으로 보입니다.
  • 🔴 필수 기능 위반 감지: Lettuce가 아닌 Redisson을 기본(Default) API로 채택하셨습니다. 과제 요구사항에 명시된 "Redisson 사용 금지 — 도전 기능에서만 허용" 조건에 위배됩니다. (상세 내용은 하단 참고)
  • ExecutorService + CyclicBarrier 동시성 테스트 코드: 현재 diff에서는 확인할 수 없습니다. (누락 여부 확인 필요)
• 도전 기능 (동시성 고도화):
  • AOP 기반 분산락 리팩토링, 비관적 락(Pessimistic Lock), Redisson 도입 및 3가지 락 방식 비교 테스트 코드가 감지됩니다. 아주 훌륭한 시도입니다! 👍

---

🟢 잘된 점

1. 체계적인 성능 비교 환경 구축: run-event-order-compare.sh 쉘 스크립트를 통해 DB를 초기화하고 각 버전별로 K6 부하 테스트를 자동화한 점이 실무적인 접근 방식과 매우 가깝습니다. 테스트의 신뢰도를 높이는 좋은 습관입니다.
2. 상세한 K6 메트릭(Metrics) 설계: 단순 성공/실패율을 넘어, Lock 실패, 품절, 중복 주문 등 실패 원인을 세분화하여 메트릭을 수집하도록 K6 스크립트를 작성하신 점이 훌륭합니다. 장애 분석(Dashboard)에 큰 도움이 되는 구조입니다.
3. 명확한 주석과 의도 표현: 코드에 주석을 통해 왜 이 전략(Fail Fast, Retry, Blocking)을 선택했는지, 각 메서드의 목적이 무엇인지 명확하게 적어두셔서 리뷰어가 의도를 파악하기 매우 수월했습니다.

---

🔴 필수 수정

1. 기본 API의 Redisson 사용 (과제 위반)

• 문제점: EventOrderController의 최상단 메인 API(/api/events/products/{productId}/orders)가 Redisson을 사용하는 createEventOrderWithRedissonLockAopBlockingWatchdog으로 매핑되어 있습니다.
• 해결: 과제의 필수 요구사항(Lettuce 사용)을 준수하기 위해, 메인 API는 Lettuce 기반의 구현체를 호출하도록 변경해야 합니다. 현재 v1~v8로 나누어 테스트하신 환경은 그대로 두시되, 최종 선택지는 Lettuce 기반의 AOP 또는 Retry/Blocking 전략 중 하나로 변경하셔야 합니다.

2. 민감 정보(Secret) 노출 위험

• 문제점: 루트 경로에 생성된 replacements.txt 파일에 soyount7509==>REMOVED_SECRET라는 내용이 커밋되어 있습니다. 이는 CI/CD 파이프라인이나 로컬 환경에서 문자열 치환을 위해 사용하신 흔적으로 보이지만, Git 이력에 민감 정보가 노출될 위험이 있습니다.
• 해결: 해당 파일을 즉시 삭제하고, .gitignore에 추가해 주세요. 만약 soyount7509가 실제 사용하시는 DB 비밀번호 등이라면 즉시 변경하시는 것을 권장합니다.

3. SQL 스크립트 변수 바인딩 미지원

• 문제점: reset-event-test.sql 파일 내에 WHERE id = ${PRODUCT_ID}와 같이 쉘 스크립트의 환경변수를 직접 참조하려는 흔적이 있습니다. 하지만 순수 SQL 파일(.sql)은 쉘 환경변수를 인식하지 못합니다.
• 해결: run-event-order-compare.sh 파일의 reset_db() 함수처럼 쉘 스크립트 내에서 히어독(Heredoc, <<SQL ... SQL)을 사용해 변수를 치환하여 DB에 전달하는 방식이 올바른 접근입니다. 미사용 SQL 파일은 정리해 주시는 것이 좋습니다.

---

🟡 권장 개선

1. 시간 단위 변경에 따른 의도치 않은 락 시간 증가 (성능 저하 가능성)

• 문제점: RedissonLock 애노테이션의 속성 이름을 waitTimeSeconds에서 waitTimeMillis로 변경하셨습니다. 이 과정에서 설정값을 이전 단위(초) 기준의 숫자에서 밀리초 단위 숫자로 수정하지 않아서, 락 대기 시간이 의도치 않게 변경되었습니다.

  // 변경 전 (최대 10초 대기)
  waitTimeSeconds = 10
  
  // 변경 후 (최대 10밀리초 = 0.01초 대기)
  waitTimeMillis = 10000 // 이렇게 수정되어야 하나, 실제 코드는 10000으로 잘 되어있는 반면
  // 다른 메서드들을 보면:
  waitTimeMillis = 5000 // 5초 대기 (정상)
  leaseTimeMillis = 3000 // 3초 유지 (정상)

• 개선 제안: 단위 변경 시 수치 값도 밀리초 기준에 맞게 정확히 조정했는지 한 번 더 확인해 주세요. 특히 테스트 결과 비교 시 이 시간 설정값이 결과에 큰 영향을 미칩니다.

2. 테스트를 위한 코드와 프로덕션 코드의 분리

• 문제점: EventOrderController에 테스트용 v1 ~ v8 메서드가 모두 노출되어 있습니다. 코드가 길어지고 유지보수성이 떨어질 수 있습니다.
• 개선 제안: 테스트 전용 API는 별도의 프로필(test 프로필 등)에서만 활성화되도록 하거나, 테스트가 완전히 종료되면 제거하는 것이 좋습니다.

  // 예시: 테스트 환경에서만 활성화
  @Profile("test")
  @RestController
  @RequestMapping("/api/events/test")
  public class EventOrderTestController { ... }

---

💡 학습 포인트

1. Distributed Lock: Redisson vs Lettuce
   • Lettuce: SETNX 기반으로 락을 구현합니다. 락 획득을 실패하면 애플리케이션 레벨에서 반복문(Spin Lock)을 돌며 재시도해야 합니다. 구현이 비교적 간단하지만, 재시도 로직 및 TTL 연장(Watchdog 등)을 직접 구현해야 하는 부담이 있습니다.
   • Redisson: pub/sub 방식을 사용하여 락 해제 이벤트

---

💬 리뷰에 대해 궁금한 점이 있나요? 코멘트에 @sparta 를 남겨보세요!
예: @sparta 이 코드에서 동시성 이슈가 발생할 수 있나요?

---

AI 리뷰는 참고용입니다. 최종 판단은 팀원이 직접 합니다.