[1단계 - DB 복제와 캐시] 조조(조은별) 미션 제출합니다. #62
Conversation
|
|
||
| ReadOnlyDataSourceRouter readOnlyDataSourceRouter = new ReadOnlyDataSourceRouter(); | ||
| readOnlyDataSourceRouter.setTargetDataSources(dataSourceMap); | ||
| readOnlyDataSourceRouter.setDefaultTargetDataSource(readDataSource); |
There was a problem hiding this comment.
기본 데이터소스를 READ로 지정한 이유가 있을까요 ? 읽기 DB는 read-only 상황에서만 접근하니 일반적으로 사용하는 기본값은 쓰기 DB가 되는게 적합해 보여서요 ~!
| import org.springframework.transaction.support.TransactionSynchronizationManager; | ||
|
|
||
| @Slf4j | ||
| public class ReadOnlyDataSourceRouter extends AbstractRoutingDataSource { |
There was a problem hiding this comment.
이 클래스명은 얼핏 보면 읽기 전용 데이터소스에만 한정되는 느낌이 드는 것 같아요. 이 클래스의 역할은 '트랜잭션' 여부에 따른 데이터소스 지정인 만큼 그 의도가 조금 더 드러났으면 좋겠어요~
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class CouponController { |
There was a problem hiding this comment.
이번 미션 요구사항 대로면 서비스 코드만 작성하면 될 것 같은데요. 컨트롤러까지 작성하신 이유가 있을까요?
|
|
||
| @NotNull | ||
| @Embedded | ||
| private IssuancePeriod issuanceDate; |
There was a problem hiding this comment.
(반영 불필요)
쿠폰 도메인에 들어가는 날짜는 "발급 날짜" 가 아니라 "발급이 가능한 날짜" 에 해당되니(시작일부터 종료일까지 발급 - 객체 생성 이 가능함) Issuance보단 Issuable이라는 단어가 조금더 변수에 적합할 것 같아요.
| private void validate(DiscountAmount discountAmount, MinOrderAmount minOderAmount) { | ||
| int discountRate = (int) ((double) discountAmount.getAmount() / minOderAmount.getAmount() * 100); | ||
| if (discountRate < 3 || discountRate > 20) { | ||
| throw new IllegalArgumentException("할인율은 3% 이상 20% 이하여야 합니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
할인 금액, 최소 주문 금액들을 모두 별도의 객체로 분리해서 역할분리를 꼼꼼하게 하신 것 같아요.
그런 맥락에서 말씀드리면 할인율에 대한 검증이 쿠폰이 해야 할 일인지 조금 더 고민해볼 수 있을 것 같아요.
우리가 실제 커머스에서 쿠폰을 사용하는 상황을 생각해보면, 쿠폰을 일단 발급을 받고 결제(사용)하는 시점에 이 쿠폰이 사용 가능한지 판단하니깐요 ~
There was a problem hiding this comment.
저도 이 로직을 Coupon에 넣었다가 나중에 제외했었는데요, 3과 20이라는 할인률을 상수로 지정할 때 이 상수가 과연 이 클래스에 적합한 상수인가? 라는 생각이 들었던 것 같습니다 ~
| @Transactional(propagation = Propagation.REQUIRES_NEW) | ||
| public <T, R> R apply(Function<T, R> function, T t) { | ||
| return function.apply(t); | ||
| } |
There was a problem hiding this comment.
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |
| public <T, R> R apply(Function<T, R> function, T t) { | |
| return function.apply(t); | |
| } | |
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |
| public <T> T supply(Supplier<T> supplier) { | |
| return supplier.get(); | |
| } |
| .orElse(findByIdWithWriteDB(couponId)); | ||
| } | ||
|
|
||
| private Coupon findByIdWithWriteDB(long couponId) { |
There was a problem hiding this comment.
레벨4 시작할 때 저희 팀에서 한창 논의하던 주제인데요, 서비스 코드에 Writer DB를 쓴다는 내용이 들어가는게 맞나? 라는 생각을 해볼 수 있겠습니다. 서비스 코드에서는 읽어온다. -> 실패하면 다시 읽어온다 -> 그래도 실패하면 예외를 던진다 의 흐름만 드러나는게 좋다고 생각합니다.
Writer DB를 사용하여 읽는 상황은 "실패" 와 관련된 것이라고 생각해서 저는 조조의 코드에 있는 WriterDBConnector와 같은 역할을 하는 클래스명을 Fallback~ 으로 지정했는데 조조의 생각은 어떤지 공유해주시면 좋을 것 같아요 ~
| show_sql: true | ||
| use_sql_comments: true | ||
| hbm2ddl.auto: validate |
There was a problem hiding this comment.
운영환경과 동일해야 한다는 요구사항이 있었지만, hbm2ddl.auto: validate으로 두지 못했어요. ddl 파일을 만들어서 시도해보았는데, 생성이 안 되더라구요. 이부분은 더 시도해보겠습니다! 편하게 리뷰주세요! 감사합니다!!
이 미션이 기존의 서비스를 개선하는 것이라면 모를까, 엔티티부터 만드는 단계인데 최초 1회는 create를 사용해도 되지 않을까요? 저는 크게 문제는 없다고 생각하는데요 ~
(오히려 테이블을 수동으로 짜면 휴먼에러로 JPA에서 지정한 설정이랑 달라질 여지가 있을수도요..?)
만약 .sql 파일을 만들어 초기에 데이터를 넣으시는 거라면 DockerFile에 COPY [sql파일 경로] /docker-entrypoint-initdb.d/ 명령을 추가하고 이미지 빌드를 하심 될거같아요 ~
|
|
||
| @Test | ||
| void 쿠폰_이름이_30자_초과이면_예외가_발생한다() { | ||
| assertThatThrownBy(() -> new CouponName("쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠폰쿠")) |
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋ 하나하나 세면서 작성하시느라 고생하셨네요 ㅋㅋㅋ string의 repeat을 사용하면 더 간단하게 가능할 것 같아요~(물론 저도 한글자씩 쳤음)
| @Transactional(readOnly = true) | ||
| public Coupon findById(long couponId) { | ||
| return couponRepository.findById(couponId) | ||
| .orElse(findByIdWithWriteDB(couponId)); |
There was a problem hiding this comment.
격리를 했으니 크게 문제되는 지점은 아니지만 개인적인 생각 하나만 남겨봅니다 ㅎㅎ 글로 남기기 애매한데 이해 안가면 댓글 남겨주세요 ~
지금 findById()의 흐름을 보면 Transactional(readOnly=true) -> Transactional 지정 X -> Transactional(readonly=false, propagation = Propagation.REQUIRES_NEW) 순으로 호출되는데요,
지금은 코드량이 매우 적어 이해하기 충분하지만 서비스 코드가 많아질 경우 Transactional을 지정하지 않은 메서드를 내부 호출하는 것이 트랜잭션 경계를 판단하는데 혼동을 줄 수 있다는 생각입니다 ㅎㅎ 추가적인 문제가 발생될 여지도 있구요 ~
(findByIdWithWriteDB()에 Transactional(propagation = REQUIRES_NEW)를 지정해도 프록시 호출이 아니니 격리가 되지도 않고, Transactional을 클래스에 붙이도록 수정했을 때도 문제가 발생할 여지가 있겠죠 ~)
따라서 중간 다리(여기서는 findByIdWithWriteDB)는 가급적 없애고, Transactional 이 붙은 메서드끼리 호출하도록 하는게 흐름 파악이 훨씬 수월할 것 같다는 생각입니다 ~
상돌🤓 안녕하세요! 조조입니다~
환경 셋팅부터 정말 쉽지 않았네요..🥲 아직 부족한 부분이 많습니다.
운영환경과 동일해야 한다는 요구사항이 있었지만,
hbm2ddl.auto: validate으로 두지 못했어요. ddl 파일을 만들어서 시도해보았는데, 생성이 안 되더라구요. 이부분은 더 시도해보겠습니다! 편하게 리뷰주세요! 감사합니다!!실행 방법
/docker디렉토리에서docker-compose up -d실행시키면 됩니다.복제 지연으로 인한 이슈 해결 - Read DB 실패 시, Write DB로 재시도 한다
우선, 운영 환경에서
쿠폰을 생성한 후 즉시 조회하는 경우가 빈번하게 발생하지 않다고 생각했어요. 보통 쇼핑몰에서 쿠폰을 뿌릴때는 특정한 기간, 다수를 대상으로 쿠폰을 풀어요. 그래서 대용량의 쿠폰은 발급 전 미리 만들어져있을 것으로 생각돼요.캐싱도 고려를 해보았는데, 현재 문제 상황은 캐시 히트가 많지 않다고 생각해요. 대사 문제도 있구요. 물론 캐시 데이터를 만료시키면 되지만, 캐시도 리소스인 만큼 자주 사용하는 경우에 적합할 것 같아요.