Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: billilge/stream-server/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| for (int attempt = 1; ; attempt++) { | ||
| try { | ||
| return transactionTemplate.execute(status -> action.get()); |
There was a problem hiding this comment.
재시도시 매번 새로운 트랜잭션을 열어서 실행하는 점 확인했습니다!
| indexes = @Index(name = "idx_items_name", columnList = "name") | ||
| ) | ||
| @DynamicUpdate | ||
| @OptimisticLocking(type = OptimisticLockType.DIRTY) |
There was a problem hiding this comment.
@Version 대신 OptimisticLockType.DIRTY을 사용해 version 컬럼 추가 없이 변경된 컬럼을 감지하는 방식으로 구현한 점 이해했습니다.
#️⃣연관된 이슈
PR #75(
feat/#74-billilge-rental-apply-return) 위에 쌓은 PR이라 base를 그 브랜치로 두었다. #75가 머지되면 base를main으로 바꾼다.🎯 해결하려는 문제가 무엇인가요?
대여 신청의 재고 차감(
findById → Item.decreaseStock → save)에 동시성 보호가 없다(508b16b에서 비관적 락 제거). 같은 물품에 신청이 동시에 들어오면 두 요청이 같은 재고를 읽고 둘 다 통과해서, 재고가 음수가 되거나 차감 하나가 사라진다(lost update).❓ 왜 해결해야 하나요?
재고가 실제 물품 수와 어긋나면 없는 물품이 대여 처리된다. 신청이 한 물품에 몰리는 순간 바로 드러나는 문제라, 대여 신청 API를 내보내기 전에 막아 둔다.
⭐ 어떻게 해결했나요?
락 — 버전 컬럼 없는 낙관적 락
ItemJpaEntity에@DynamicUpdate+@OptimisticLocking(type = OptimisticLockType.DIRTY)를 붙였다.update items set count=? where id=? and count=?(읽은 값)로 나간다. 그 사이 재고가 바뀌었으면 0건이 되어OptimisticLockingFailureException이 난다.Item, DB 스키마,ItemRepositoryImpl은 바뀌지 않는다.재시도 —
core:common의LockExecutorlockExecutor.executeOptimistic(action)은 시도마다 새 트랜잭션을 열고 action 전체를 실행한다.OPTIMISTIC_LOCK_CONFLICT(409)로 실패한다.IllegalStateException을 던진다. 같은 트랜잭션에서 재시도하면 1차 캐시와 REPEATABLE READ 스냅샷이 예전 값을 계속 돌려줘서 매번 충돌하기 때문이다.RentalApplyUseCase@Transactional을 떼고 본문 전체를lockExecutor.executeOptimistic(() -> { ... })로 감쌌다.컨벤션 문서
core:common에spring-context·spring-tx허용 (architecture.md2·2-1·4-2·7절,00-index.md)@Transactional대신LockExecutor안에서 Service를 조합한다 (architecture.md6-1절,coding-style.md2-9절)테스트 (Testcontainers MySQL, Docker가 없으면 건너뜀)
LockExecutorTest7개: 재시도, 최대 횟수, 다른 예외는 재시도하지 않음, 트랜잭션 안 호출 거부, 대기 중 인터럽트ItemOptimisticLockTest5개: 동시 차감 충돌, SQL 직접 수정 감지, 다른 컬럼 동시 수정은 둘 다 반영,LockExecutor재시도 성공, OSIV(EntityManager가 스레드에 묶인 상태)에서도 재시도 성공RentalApplyConcurrencyTest2개: 재고 5개에 20건 동시 신청 → 초과 차감 없음·이력 수 = 차감 수 / 같은 회원이 같은 대여품 5건 동시 신청 → 대여 중 이력 1건MySqlJpaTest,MySqlIntegrationTest)를 두고, 컨테이너는 tmpfs + 디스크 동기화 끔으로 설정했다.🧩 이 PR의 한계 & 트레이드오프
@Version+ API로 버전 주고받기를 도입한다.core:common이 순수 Java가 아니게 된다. 허용 범위를spring-context·spring-tx로 문서에 못 박았지만, 이를 검사하는 ArchUnit 규칙은 없어서 리뷰로 지켜야 한다.core:common에 slf4j가 없어서, 마지막 409만GlobalExceptionHandler가 warn으로 남긴다.@JdbcTypeCode처럼 이미 쓰고 있는 범주)java-test-fixtures로 합칠 수 있지만 새 빌드 방식이라 이번에는 넣지 않았다.⛓️ 기존 기능에 미치는 영향
@Transactional에서LockExecutor로 바뀐다. 동작과 원자성은 같고, 충돌하면 재시도·409가 추가된다.itemsUPDATE는 바뀐 컬럼만 갱신한다(@DynamicUpdate). 지금Item을 저장하는 경로는 재고 차감 하나뿐이다.core:common에 의존하는 모듈의 런타임에spring-tx·spring-context가 들어온다. 앱 전체(bootstrap)에는 원래 있던 의존이다.spring-boot-testcontainers,spring-jdbc(bootstrap 테스트용).🔀 Edge Case & 실패 시나리오
OPTIMISTIC_LOCK_CONFLICT("잠시 후 다시 시도해 주세요")ITEM_OUT_OF_STOCKRENTAL_ITEM_DUPLICATED. 중복 대여 경합도 함께 막힌다.count를 직접 바꾸는 중에 신청 → 충돌로 감지하고 재시도LockExecutor를 트랜잭션 안에서 호출 →IllegalStateException(코드 실수라 500)JpaTransactionManager가EntityManager를 비우므로 재시도는 DB에서 새로 읽는다(테스트로 확인)📋 검토한 대안과 선택 이유
LockExecutor로 감싸기: web 계층에 락 로직이 들어가서 제외했다.TransactionTemplate을 직접 쓰거나, 트랜잭션 부분을 별도 빈으로 분리: 각각 새 패턴·클래스 분리가 필요하다. 대신LockExecutor가 트랜잭션까지 열게 하고core:common에spring-tx를 허용했다. 호출부는 람다 안에서 Service를 조합하기만 하면 된다.@Version컬럼: 지금save가 도메인 → 새 엔티티 → merge 방식이라 도메인Item이 버전을 들고 있어야 하고, 마이그레이션도 필요하다. 운영진이 SQL로 재고를 고칠 때 버전도 올려야 한다. DIRTY는 이 셋이 모두 필요 없어서 택했다. 화면 단위 충돌 감지가 필요해지면 그때 도입한다.508b16b에서 뺀 방식이다.💬 리뷰 포인트
core:common에 Spring 의존 허용 — 컨벤션 변경이라 합의가 필요하다 (architecture.md변경분)LockExecutor의 트랜잭션 경계와RentalApplyUseCase에서@Transactional을 뗀 부분