Skip to content

Conversation

@ba2slk
Copy link

@ba2slk ba2slk commented Oct 25, 2025

  • User -> Member 로 변경 + 인가가 필요한 곳에 User Details를 적용한 코드가 다른 로컬에 있어서, 다음 PR 때 꼭 반영하겠습니다!

Copy link

@Hoyoung027 Hoyoung027 left a comment

Choose a reason for hiding this comment

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

코드가 전반적으로 너무 깔끔해서 읽기 편했습니다! 저도 이렇게 작성하고 싶어요,,,
내 꿈은 너야 배승식

} else if (payableItem instanceof UserOrder userOrder) {
restoreStock(userOrder);
}
}

Choose a reason for hiding this comment

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

instanceof로 분기 처리를 하는건 굉장히 세련된 방법이라고 느껴지네요! 참고하겠습니다 👍👍

Copy link
Author

Choose a reason for hiding this comment

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

구현체로 분기를 나눌 때 좋은 것 같더라구요!

public interface SeatRepository extends JpaRepository<Seat, Long> {
List<Seat> findByScreen_Id(Long screenId);

@Lock(LockModeType.PESSIMISTIC_WRITE)

Choose a reason for hiding this comment

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

Pessimistic Lock의 경우 Spin Lock 방식이라 lock을 획득하지 못할 경우 이론상 무한 대기가 일어날 수 있는 것으로 알고 있습니다. (물론 흔히 발생하진 않을거 같지만요,,)

위의 상황을 방지하고자 저의 경우에는 query 자체에 timeout을 걸어서 lock 획득 실패로 처리하도록 구현했고, 반드시 수행되어야 하는 작업이라면 재시도 로직을 별도로 작성하였습니다.

@Lock(LockModeType.PESSIMISTIC_WRITE)
@QueryHints({
        @QueryHint(name = "jakarta.persistence.lock.timeout", value = "2000")
})
List<Seat> findWithLockByIdIn(@Param("seatIds") List<Long> seatIds);

위 코드처럼 바꾸면 query를 날릴 때 timeout이 발생하더라구요. 참고해보셔도 좋을 것 같아 남겨봅니당!

} catch (EntityNotFoundException | IllegalStateException e) {
return ResponseEntity.badRequest().body(null);
}
}

Choose a reason for hiding this comment

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

저의 경우에는 에러 핸들링을 Service 단에서 처리하는데 승식님은 controller에서 하시는군요!
에러 핸들링을 어디에서 해야하는건지 저도 결정을 잘 못하고 있어서, 혹시 controller에서 처리했을 때의 장점이 따로 있으셔서 진행하시는건지 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

저도 Service 단에서 주로 에러를 처리하는 편인데, 이때는 조금 급하게 짜느라 컨트롤러에 욱여 넣은 것 같아요.. 이번 PR에서 예외 처리 책임 분리했습니다!

payableItem.confirm();

return response;
}

Choose a reason for hiding this comment

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

저의 경우에는 reservation은 "/api/reservations"에 해당하는 api에서 redisson을 활용한 lock을 걸어 한 좌석을 여러 사용자가 동시에 예약하지 못하도록 했으며, 매점 주문의 경우에는 결제 시점인 "/api/payments/confirm"가 일어날 때 pessimistic lock을 걸어 동시성을 제어했습니다.

좌석은 예약 시작 단계에서 동시성 제어가 필요하고, 매점의 경우에는 결제 시점의 재고 관리를 고려한 판단이었습니다.

승식님의 경우 reservationSeat에 unique 제약 조건을 걸어 이미 예약 단계에서도 중복 예약이 발생하지 않도록 하고 계신걸로 보이는데, 해당 방식과 제 방식 사이에 어떤 장단점이 있을지 승식님 의견이 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

저는 트랜잭션 롤백 시에 일관성이 보장된다는 점이 장점이라고 생각했습니다. redisson 방식은 서버 인스턴스가 여러 대일 때에도 일관되게 동시성 제어가 가능하다는 점에서 강점이 있을 것 같습니다. 반대로 unique 제약 방식은 단순하지만 DB 단일 노드에 의존적이라는 점에서 단점이 있는 것 같아요! 서버 아키텍처에 따라서 각각 장단점이 있지 않을까 생각합니다..!

InstantPaymentResponseDTO response = paymentClient.instant(
orderUuid.toString(),
paymentRequest
);

Choose a reason for hiding this comment

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

저는 외부 API 호출이 일어나는 메소드에서 의도적으로 @Transactional 어노테이션을 제거했었습니다.

외부 API의 동작에 따라서 서비스 내부, 외부의 데이터 간 차이가 발생할 수 있을 것이라고 생각했습니다. 예를 들어 외부에서는 결제 처리가 되었는데, 내부 후반부 로직에서 에러가 터지면 내부 DB는 해당 결제 내역 자체를 반영하지 못할 수 있을 것입니다.

그래서 저는 외부 API 호출 결과에 따라서 내부 DB 변동이 필요한 메소드를 다른 bean의 메소드로 분리하고 별도로 @transactional을 달아서 짧은 트랜젝션을 여러개 만들었습니다. 이렇게 하면 로직 후반부에 에러가 나더라도 외부 API 결과에 대해 업데이트 해야할 부분을 다 날리지는 않을 것 같아요.

저도 어떤 식으로 작성해야할지 정답은 모르겠어서, 승식님 의견이 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

트랜잭션을 분리하면 원자성을 보장하기 위한 구현 부담이 커질 것으로 생각해서, 일단은 외부 API 호출까지 트랜잭션으로 감싼 형태로 구현했습니다. 하지만 지금 제 코드의 경우에 외부 API 응답이 실패했을 때에 대한 명시적인 예외 처리나 복구 로직이 없기 때문에 호영님이 하신 것처럼 여러 트랜잭션으로 나누어 처리하는 방법이 더 유효해보입니다. 저도 더 많이 고민해봐야겠네요... 좋은 인사이트 감사합니다!!

@Gothax Gothax merged commit 4f7aee6 into CEOS-Developers:ba2slk Oct 30, 2025
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.

3 participants