결제 시스템 취소, 스케쥴러 충돌 문제

[wlgns12370]

배경

PR #28(ReservationService) 리뷰 중 발견한 문제입니다.

한 줄 요약: 재고·슬롯 수량은 안전하게 잠그고 있지만, "지금 이 예약을 취소 처리해도 되는지"를
판단하는 예약 상태 자체는 잠그지 않아서, 취소와 자동만료가 겹치면 재고가 실제보다 많이
복원되는 문제가 있습니다.

구체적으로 어떤 상황인가요?

사용자가 예약을 취소하는 순간과, 픽업 홀드가 지나 자동으로 정리하는 스케줄러(60초마다 실행)가
같은 예약을 동시에 처리하면 다음과 같은 일이 벌어질 수 있습니다.

1. 사용자와 스케줄러가 거의 동시에 같은 예약을 조회 → 둘 다 "아직 취소 안 됨"으로 봄
2. 사용자가 먼저 취소 처리, 재고 +1 복원, 커밋
3. 스케줄러도 (자기가 조회했던 시점 기준으로) 재고 +1을 한 번 더 복원

결과적으로 재고가 실제보다 2자리 더 많아집니다. 재고·슬롯 테이블에는 discussion#13(방안 A,
Row Lock)에 따라 PESSIMISTIC_WRITE 락이 정확히 걸려 있지만, "그 락을 실행할지 말지"를
결정하는 예약(Reservation) 행 자체는 findById()로만 읽고 아무도 잠그지 않기 때문입니다.
재고를 잠가도, 재고를 복원할지 말지 정하는 조건이 안전하지 않으면 락은 의미가 없습니다.

용어 정리: 여기서 "스케줄러"는 ReservationExpirySweeper(@Scheduled(fixedDelay = 60_000), 60초 주기)를 말합니다. 결제 성공 여부가 아니라 Reservation.expiresAt(픽업 홀드
만료 시각)이 지났는지만 봅니다. 결제(Stripe) 도메인은 이 PR 시점엔 아직 없어서, "결제 확인용
스윕"은 이후 결제 분리(discussion#16)에서 별도로 다룰 사안입니다. 혼동을 피하기 위해 여기서는
"픽업 홀드 만료 스윕"으로 부릅니다.

이번 논의의 범위

• 재고·슬롯의 락 방식(discussion#13)은 이미 확정된 사안이라 그대로 유지합니다. 이번 논의는
  그 위에서 아직 안 잠긴 Reservation 행 하나를 어떻게 보호할지로 범위를 한정합니다.
• Reservation은 취소(DELETE /reservations/{id}) · 조회(GET /reservations/{id}) ·
  픽업 검증(POST .../redeem) · 자동만료(스케줄러), 이렇게 4개 경로에서 각각 별도
  트랜잭션으로 접근됩니다.
• Reservation.cancel()은 이미 CANCELLED/COMPLETED 상태면 예외를 던지긴 하지만, 이
  체크는 각 트랜잭션이 자기가 조회를 시작할 때 읽어온 값을 기준으로 판단하는 거라, 상대방이
  먼저 커밋해버려도 그 사실을 알아채지 못합니다.

---

문제 재현

사용자가 취소 버튼을 누르는 순간과, 스윕러가 같은 예약을 "정리 대상"으로 집는 순간이 겹치는
경우입니다.

sequenceDiagram
    participant U as 사용자<br/>(취소 요청)
    participant SW as 만료 체크 스케쥴러<br/>(60초 주기)
    participant DB_R as DB · reservations
    participant DB_C as DB · 재고 행

    U->>DB_R: SELECT status (락 없음) → RESERVED
    SW->>DB_R: SELECT status (락 없음) → RESERVED
    Note over U,SW: 둘 다 "아직 취소 안 됨"으로 판단
    U->>DB_R: UPDATE status = CANCELLED
    U->>DB_C: 🔒 락 획득 → 재고 +1
    U-->>U: 커밋 (락 해제)
    SW->>DB_C: 🔒 대기하다 락 획득 → 재고 +1 (또!)
    SW->>DB_R: UPDATE status = CANCELLED (덮어씀)
    Note over DB_C: 결과: 재고가 1+1,<br/>실제보다 1 더 많아짐

Loading

재고 행 자체는 PESSIMISTIC_WRITE라 두 복원이 순서대로 처리되긴 하지만, "복원해도 되는지"를 각자
잘못 판단한 채로 순서대로 실행되는 것뿐이라 결과적으로 이중 복원을 막지 못합니다.

---

해결방안

방안 1: 비관적 락 (Reservation 조회에도 PESSIMISTIC_WRITE)

cancelReservation / redeem / expireOverdueReservations에서 findById 대신 SELECT ... FOR UPDATE로 예약 행을 잠그고 시작합니다. 잠금 조회는 스냅샷이 아니라 현재 커밋된 값을 읽으므로,
뒤에 도착한 트랜잭션은 락을 기다렸다가 이미 CANCELLED로 바뀐 최신 상태를 보게 되고, 기존
cancel()의 상태 체크가 그대로 ALREADY_CANCELLED로 막아줍니다 — 새 예외 타입 없이 기존 도메인
예외 경로를 재사용할 수 있습니다.

다만 재고 → 슬롯 순서로만 정의돼 있던 락 획득 순서에 Reservation이 추가되므로, 데드락을 피하려면
전 경로에서 "예약 → 재고 → 슬롯" 순서를 새로 통일해야 합니다.

방안 2: 낙관적 락 (@Version 컬럼)

Reservation에 @Version private Long version;을 추가합니다. 평소에는 아무 비용이 없다가, 커밋
시점에 JPA가 버전이 그사이 바뀌었는지 자동으로 검사해 바뀌었으면
ObjectOptimisticLockingFailureException을 던집니다. 늦게 커밋하려는 쪽만 실패하므로 재고 복원은
정확히 한 번만 일어납니다.

---

비교

관점	방안 1: 비관적 락	방안 2: 낙관적 락(@Version)
충돌 없는 정상 경로 비용	매 취소·redeem·만료마다 락 획득/해제 비용 상시 발생	버전 컬럼 비교만 추가, 사실상 무비용
충돌 시 결과	뒤 트랜잭션이 대기 후 최신 상태를 보고 기존 ALREADY_CANCELLED 예외로 자연스럽게 막힘	늦은 트랜잭션이 OptimisticLockException — 서비스 레이어에서 별도로 잡아 처리 필요
데드락 위험	Reservation이 락 대상에 추가되며 재고·슬롯과의 락 순서 규칙을 새로 정의해야 함	락을 아예 안 걸어 데드락 리스크 없음
구현 변경 범위	리포지토리에 findByIdForUpdate 추가, 호출부 3곳 교체	엔티티에 컬럼 추가 + 마이그레이션, 충돌 예외 핸들링 추가
이 레이스의 발생 빈도	항상 안전(락 방식이라 확률과 무관)	낮은 충돌 확률을 전제로 함 — 실제로 잦다면 재시도 로직까지 필요해짐

---

의견 수립

이 문제를 푸는 방향으로 방안 2(낙관적 락)를 제안합니다. 이유는 다음과 같습니다.

1. 이 레이스가 실제로 발생하려면 "취소 요청"과 "60초 스윕 주기"가 초 단위로 겹쳐야 하는데, 이건
   2시간 픽업 홀드 중 마지막 60초 안팎의 아주 좁은 창에서, 그것도 사용자가 하필 그 순간 취소를
   눌러야만 생깁니다. 이 정도로 드문 경합을 막으려고 Reservation의 모든 취소·redeem·조회 경로에
   상시 락 대기 비용을 지불하는 건 과하다고 봤습니다.
2. 낙관적 락은 "거의 항상 공짜, 충돌 났을 때만 비용"이라 이 케이스의 확률 분포(거의 0, 아주 가끔
   1)에 더 잘 맞습니다.
3. 재고·슬롯처럼 항상 잠가야 하는 자원(모든 예약 생성이 반드시 거치는 경로)과 달리, Reservation
   취소·만료는 상대적으로 드문 경로라 락 순서 규칙을 새로 늘리는 것보다 부담이 적은 쪽을
   택했습니다.

다만 이건 "충돌 확률이 낮다"는 가정에 기대고 있어서, 그 가정 자체가 맞는지 팀 의견이 필요합니다.

---

의견 요청

궁금한 점은 다음과 같습니다.

1. 이 레이스의 발생 확률을 "낮다"고 보는 판단이 맞는지 — 스케줄러 주기(60초)나 실제 취소 패턴을
   감안했을 때 이 정도로 안심해도 되는 수준인지 궁금합니다.
2. OptimisticLockException이 실제로 발생했을 때, 취소 요청 쪽 사용자에게는 어떤 응답을 주는 게
   맞을지(예: "이미 만료 처리된 예약입니다" 안내 vs 조용히 성공 처리) 의견을 듣고 싶습니다.
3. 이번 레이스는 Reservation 상태 전이에 국한된 문제인데, 같은 종류의 check-then-act 레이스가
   있는 노쇼 제한 체크(assertNoShowLimitNotExceeded)도 같은 낙관적 락으로 커버할 수 있는지, 아니면
   별도 처리가 필요한지 궁금합니다.
4. 프로덕션에서 이 충돌이 실제로 얼마나 발생하는지 관측할 방법(로그/메트릭)을 이번에 같이 넣을지,
   아니면 발생 시점에 추가할지 궁금합니다.

[GyunHeee]

질문 남긴 지 얼마 안 됐는데, 스스로 다시 정리해봤습니다.

1. "낮은 확률" 판단, 다시 봐도 맞을까

"취소 요청"과 "60초 스윕 주기"가 독립적인 무작위 사건이라면 확률이 낮다는 계산이
맞겠지만, 실제로는 독립이 아닐 수도 있겠다 싶습니다. 만료 임박 알림을 보고 "어차피
곧 끝나는데 그냥 취소하자"는 식으로 행동하는 사용자가 있다면, 취소 시점이 만료
시점 근처로 몰리면서 두 사건이 상관관계를 갖게 됩니다. 그러면 나이브하게 계산한
확률보다 실제 충돌 빈도가 높을 수 있습니다.

다만 이 불확실성이 방안 2를 재고할 이유는 아니라고 봅니다. 낙관적 락은 예측이
틀려서 충돌이 예상보다 잦아도 "재시도해야 할 예외가 좀 더 자주 난다"로 끝나지,
재고가 잘못 복원되는 데이터 정합성 문제로 이어지지 않습니다. 확률 추정이 틀렸을
때의 대가가 작다는 게 방안 2를 미는 더 중요한 이유인 것 같습니다. 정확한 빈도는
3번에서 관측으로 실측하면 될 것 같습니다.

2. 취소 요청 쪽 사용자에게 어떤 응답을 줘야 할까

OptimisticLockException을 그대로 노출하지 않는 쪽으로 정리했습니다. 서비스
레이어에서 캐치한 뒤, 예약을 다시 조회해서 현재 상태를 확인하고:

• 이미 CANCELLED/EXPIRED로 넘어가 있다면 → 원했던 결과(예약 비활성화)는
  이미 달성된 상태이니, 에러가 아니라 **취소 성공(멱등 응답)**으로 처리하는 게
  맞다고 봅니다. 사용자 입장에서는 "취소해달라"는 의도가 이미 실현된 거니까요.
• 그 외 예기치 않은 상태라면 일반적인 "다시 시도해주세요" 메시지로 처리합니다.

Reservation.cancel()의 기존 상태 체크(ALREADY_CANCELLED 등)를 재사용할 수
있는지 먼저 보고, 안 되면 이 분기만 서비스 레이어에 추가하면 될 것 같습니다.

3. 관측(로그/메트릭), 지금 넣을까 나중에 넣을까

지금 같이 넣는 쪽으로 정리했습니다. 1번에서 얘기했듯 방안 2를 선택한 근거 자체가
"충돌이 드물 것"이라는 가정인데, 그 가정을 검증할 방법이 없으면 이 판단이 맞았는지
알 도리가 없습니다. 최소한 ObjectOptimisticLockingFailureException을 캐치하는
지점에 카운터 하나(예: reservation.cancel.optimistic_lock_conflict, 경로별
태그 포함)만 추가해도 충분할 것 같습니다. 코드 몇 줄이면 되고, 사고 나고서
추가하는 것보다 지금 넣는 게 훨씬 쌉니다.