[1단계 - DB 복제와 캐시] 이상(이건상) 미션 제출합니다. #76
Conversation
| validateNameLength(name); | ||
| validateDiscountAmountRange(discountAmount); | ||
| validateDiscountAmountUnit(discountAmount); | ||
| int discountPercent = (discountAmount * 100) / minimumOrderPrice; | ||
| validateDiscountPercent(discountAmount, discountPercent); | ||
| validateMinimumOrderPriceRange(minimumOrderPrice); | ||
| validateIssueDateTime(issueDate, expirationDate); |
There was a problem hiding this comment.
따로 객체로 분리하지 않은 이유가 있으신가요? 그리고, 구현해주신 기능에 테스트 코드를 작성해주세요.
There was a problem hiding this comment.
검증 로직이 간단하고 쿠폰 엔티티와 직접적으로 연관되어 있어서 분리할 필요성을 못느꼈어요. 테스트는 중요하니 추가할게요 😇
| @Column(name = "discount_percent") | ||
| private Integer discountPercent; |
There was a problem hiding this comment.
discountPercent는 discountAmount와 minimumOrderPrice에 따라서 유동적으로 바뀌는 값인데, 데이터베이스에 직접 저장하신 이유가 있으신가요?
There was a problem hiding this comment.
역정규화의 일환으로 쿠폰 생성시 함께 데이터베이스에 저장해두고, 조회할 때 매번 계산하지 않도록 하여 조회 응답 성능을 올릴 수 있습니다. 이벤트로 상품 주문시 적용 가능한 쿠폰을 조회할 때 상품의 금액을 기준으로 최소 주문 금액에 더하여 n% 이하의 할인 쿠폰만 적용 가능하다면 바로 필터링 가능할 것 같아요.
| public Coupon(Integer discountAmount, Integer minimumOrderPrice) { | ||
| this("쿠폰", discountAmount, Category.FASHION, minimumOrderPrice, LocalDate.now(), LocalDate.now().plusDays(7)); | ||
| } |
There was a problem hiding this comment.
테스트에 작성돼있던 생성자인데 그 코드에 맞춰서 만들었네요. 목적에 맞게 수정하면 될 것 같아요.👍
|
|
||
| private void validateDiscountPercent(Integer discountAmount, int discountPercent) { | ||
| if (discountPercent < MIN_DISCOUNT_PERCENT || MAX_DISCOUNT_PERCENT < discountPercent) { | ||
| System.out.println("discountPercent = " + discountPercent); |
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(name = "coupon_id") | ||
| private Long couponId; |
There was a problem hiding this comment.
member coupon인데 member의 식별자도 관리해야하지 않을까요?
| @Transactional(readOnly = true) | ||
| public Coupon getCoupon(Long id) { | ||
| log.info("CouponService.getCoupon Transaction readOnly {}", TransactionSynchronizationManager.isCurrentTransactionReadOnly()); | ||
| if (cache.containsKey(id)) { | ||
| return cache.get(id); | ||
| } | ||
| return couponRepository.findById(id).orElseThrow(); | ||
| } |
There was a problem hiding this comment.
좋은 근거를 가지고 선택을 해주셨네요. 👍👍
임시 저장소를 사용하는 것은 말씀주신대로 성능향상의 기회가 있습니다. 다만, 단순 복제지연을 해결하기 위한 방법으로는 비용이 비싸다고 생각합니다. 인프라 구축 및 관리 비용, 임시 저장소에 접근하기 위한 코드 복잡도 향상, 다른 개발자로 하여금 "왜 여기서는 캐싱을 수행했지?"를 생각하게하는 비용 정도가 생각나네요.
따라서, 이상의 선택이 설득력을 가지기 위해서는 누군가(관리자, 사용자)가 쿠폰을 생성했을 때, 대량의 사용자가 쿠폰을 다시 실시간으로 조회해야하는 상황일 것 같아요. 이러한 경우라면 캐싱을 수행하는 것이 납득은 가지만 현재 구현은 애매한 지점이 있습니다.
- 모든 생성된 쿠폰은 인메모리에 적재되어야 하는가?
- 줘도 안가지는 쿠폰은 캐시 히트가 많을까? ex) 위브 담배 사주기 쿠폰
- 캐싱된 쿠폰은 언제 만료되는가?
- 쿠폰이 삭제되거나 업데이트되는 경우 어떻게 동기화가 이루어지는가?
- 데이터베이스 커밋 이전에 캐싱이 수행되면, 다른 사용자(스레드)가 캐시를 보고 데이터를 가져갔는데, 커밋이 실패하거나, 트랜잭션이 롤백된다면?
- 서버가 2대 이상인 환경에서는 복제지연이 다시 발생하지 않는가?
- docker-compose로 레디스를 사용했는데 ConcurrentHashMap을 사용하는 인지부조화(억까)
- 캐시 미스가 발생하면, 복제지연이 다시 발생할 수 있는가? 그렇다면 다른 개발자가 복제지연이 발생한다는 사실을 몰라도 괜찮은것이 장점은 아니지 않을까?
- 무조건 모든 생성 쿠폰이 인메모리에 적재되는 방식이라면 cache.get(id)을 바로 리턴하면 되지 않는가?
- 그렇다면 Transactional을 걸어서 불필요하게 DB 커넥션을 점유해야하는가?
- 캐싱을 처리했으면 findById에서 source로 조회해도 부하가 발생하지 않지 않은가? replica는 복제지연이 발생하는 구간이 아닌가?
그리고 이상이 생각하시는 이 방식의 단점이 궁금해요. 제가 생각하는 가장 중요한 것은 "무엇을 선택했을 때, 무엇을 잃었는가?" 입니다. 적어도 선택한 방식으로 인해 내가 지불해야하는 대가가 무엇인지는 정확히 알아야한다고 생각하기 때문입니다.
| Coupon coupon = new Coupon(1000, 10000); | ||
| couponService.create(coupon); | ||
|
|
||
| Thread.sleep(2000); // 복제 지연 무시. |
There was a problem hiding this comment.
복제지연 해결해주셨는데 Thread.sleep을 수행하신 이유가 있으신가요? 매번 테스트에서 2초 소모해야하는 상황이네요. 🥲
| @Component | ||
| public class DatabaseCleaner { | ||
|
|
||
| @PersistenceContext | ||
| private EntityManager entityManager; | ||
|
|
||
| @Transactional | ||
| public void execute() { | ||
| entityManager.createNativeQuery("SET FOREIGN_KEY_CHECKS = 0").executeUpdate(); | ||
| clearCoupon(); | ||
| entityManager.createNativeQuery("SET FOREIGN_KEY_CHECKS = 1").executeUpdate(); | ||
| } | ||
|
|
||
| private void clearCoupon() { | ||
| entityManager.createNativeQuery("TRUNCATE TABLE coupon").executeUpdate(); | ||
| } | ||
| } |
| @Test | ||
| void 복제지연테스트() { | ||
| Coupon coupon = new Coupon(1000, 10000); | ||
| couponService.create(coupon); | ||
| Coupon savedCoupon = couponService.getCoupon(coupon.getId()); | ||
| assertThat(savedCoupon).isNotNull(); | ||
| } |
There was a problem hiding this comment.
미션처럼 사용자가 의도적은 딜레이를 준 상황이 아닌 경우에는 복제지연을 어떻게 해결할 수 있을까요?
https://repost.aws/ko/knowledge-center/rds-mysql-high-replica-lag
안녕하세요 아톰 만나서 반갑습니다 😄
1단계는 source 와 replica 사이에 복제 지연 1초가 발생하는 상황을 어떻게 해결할 것인가 하는 미션이었어요.
첫 번째로 가장 단순히 처리하는 방법은 source DB에 CUD 하는 작업 후에 Thread.sleep()을 사용해서 replica가 복사하는 시간을 벌어주는 방법이라고 생각합니다. 이 방법을 사용하면 복제하는데 걸리는 시간이 예상과 달라질 수 있다는 점과 source DB에 쓰는 요청의 처리 시간이 늘어난다는 단점이 있습니다.
두 번째는
@Transactional바탕으로 한 쓰기 작업은 source DB에 보내고,@Transactional(readOnly = true)바탕의 작업은 replica에 보내는데, 존재하지 않는 경우는 복제 지연을 감안해서 source DB에서 다시 조회하여 재시도 처리를 하는 방법이 있습니다. 그래도 없는 경우에는 예외를 던지고요. 하지만 이 경우는 현재처럼 복제 지연이 고정적으로 무조건 발생하는 상황에서 source로의 요청이 매번 발생하게 돼서 요청의 처리 시간이 늘어나고, source의 부하가 늘어납니다.세 번째는
getCoupon메서드에서Transactional(readOnly = true)옵션으로 인해서 replica에 요청을 보내게 되는데, 복제 지연으로 인해 조회가 되지 않는 경우 새로운 트랜잭션을 열어서 조회하는 방법이 있습니다.@Transactional(propagation = Propagation.REQUIRES_NEW)옵션을 사용해서 readOnly를 사용하지 않고 조회하면 됩니다. 하지만 같은CouponService클래스 내에서 메서드를 선언하면 AOP로 동일한 프록시 객체를 사용하게 되고, readOnly = true로 열린 트랜잭션을 사용하게 되어 의도한 동작이 제대로 수행되지 않습니다. 그래서 새로운 클래스로 분리하여 메서드를 선언하고getCoupon에서 없는 경우 호출하도록 작성해야 합니다. 이 경우에는 단순 클래스 분리로 합리적인 수준에서 문제를 해결할 수 있지만, 복제 지연 시간이 실제 환경에서는 가변적일 수 있다는 점(복제 지연이 발생하지 않을 수도 있다는 점)과 복제 지연 만을 위해 클래스를 분리하는 것은 다른 개발자로 하여금 이 클래스가 필요한 원인이 뭘까? 라는 의문을 가지게 할 수 있다고 생각합니다.그래서 저는
create메서드로 쿠폰 객체를 저장할 때CouponService에 Map으로 쿠폰을 저장해두고getCoupon메서드에서 쿠폰을 조회할 때 캐시에 존재하는 경우 인메모리 캐시에서 조회하여 반환하도록 했습니다. 이 방법의 장점으로는 현재 발생하는 복제 지연과 상관 없이 바로 조회할 수 있어서 응답이 빠르고, 코드 또한 다른 개발자들이 본다고 했을 때 어떤 이유로 했는지 몰라도 '캐싱을 하고 반환해서 응답 속도를 올리려고 했구나' 정도로 이해 가능하다는 장점이 있습니다.설명이나 코드가 잘못 작성 되었거나 부족한 부분이 있다면 리뷰 주세요😇
리뷰 잘 부탁드립니다. 🙇🏽♂️