-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[1단계 - 복제 지연] 몰리(김지민) 미션 제출합니다. #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 몰리!!
우테코 마지막 미션의 리뷰이가 몰리여서 정말 반갑네요!!! ㅎㅎㅎ
저도 많이 배울 수 있을 것 같아 기대됩니다 🔥
프로젝트에서는 신뢰 기반 리뷰도 참 많이 많이 했지만… 미션 리뷰는 신뢰 0으로 갈게요 😝
마지막 미션 화이팅 몰뤼~~ 🔥
public Coupon getCouponFromWrite(final Long id) { | ||
return transactionRouter.routeWrite(() -> couponRepository.fetchById(id)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransactionRouter 객체를 사용하는 대신, Propagation.REQUIRES_NEW 속성의 메서드를 만들어서 실행하면 어떻게 될까요? 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransactionRouter라는 객체가 아닌 내부에서 해당 메서드에 Propagation.REQUIRES_NEW 속성을 주게 되면, 테스트에 실패하는데요.
그 이유는 동일한 클래스 내에서 @transactional이 적용된 내부 메서드를 호출하는 경우, 호출되는 메서드는 Proxy 객체를 거치지 않고 직접 호출되기 때문에, Transactional Interceptor가 적용되지 않고 결과적으로 @transactional이 어노테이션이 무시되게 됩니다.
결국 getCoupon의 readOnly 트랜잭션이 그대로 유지되고 write DB로의 전환이 일어나지 않아 테스트에서도 실패하게 됩니다!
@Column(name = "name", nullable = false) | ||
private Name name; | ||
|
||
@Column(name = "discount_mount", nullable = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yml 파일에 다음과 같은 설정이 적용되어 있었기 때문이네요!
spring:
jpa:
hibernate:
naming:
implicit-strategy: org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl
# 명시적으로 이름을 지정하지 않았을 때 자동으로 이름을 생성하는 전략 - 카멜케이스를 그대로 사용하는 설정 값으로 되어 있음
physical-strategy: org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
# 실제 DB의 테이블이나 컬럼이 생성될 때 사용되는 이름을 결정하는 전략 - 논리적 이름을 그대로 사용하는 값으로 설정 되어 있음
따라서 물리명을 카멜 케이스를 언더 스코어로 변환해주는 CamelCaseToUnderscoresNamingStrategy 로 변경하였습니다!
public Coupon( | ||
final String name, | ||
final Long discountAmount, | ||
final Integer minimumOrderAmount, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
금액을 의미하는 필드들의 타입은 long으로 일치시키면 더 좋을 것 같은데, 몰리 생각은 어떤가요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호! 놓쳤던 부분이네요 반영하겠습니다 🫡
|
||
public void validateDiscountAmountRange(final Long discountAmount) { | ||
if (discountAmount > MAXIMUM_DISCOUNT_AMOUNT || discountAmount < MINIMUM_DISCOUNT_AMOUNT) { | ||
throw new IllegalArgumentException("할인 금액은 1,000원 이상 10,000원 이하만 가능합니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메시지에도 상수를 사용할 수 있을 것 같아요. 전체적으로 반영해볼 수 있을까요?
} | ||
|
||
@Bean | ||
@DependsOn({WRITE_DATASOURCE, READ_DATASOURCE}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DependsOn
애너테이션을 사용한 이유가 궁금해요!
해당 애너테이션을 사용하지 않으면 어떤 문제가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DependsOn
을 통해 빈의 생성 순서를 제어할 수 있습니다.
@DependsOn
에 명시된 빈은 현재 빈이 의존하는 빈입니다. 지정된 모든 빈은 이 빈보다 먼저 컨테이너에 의해 생성되도록 보장됩니다.
만약 해당 어노테이션을 사용하지 않는다면 ReadDataSource
, WriteDataSource
가 빈 등록되어 있지 않는 시점에서 호출을 하게 될 수 있는 위험이 존재합니다. 따라서 @DependsOn
을 사용하여 빈 의존 관계를 명시적으로 선언하고 안전한 초기화를 보장하도록 하였습니다!
그런데 기존 코드의 routeDataSource에서는 빈 주입으로 각 데이터 소스를 받는 것이 아닌, 직접 메서드를 호출하여 사용하였기 때문에 @DependsOn
의 의미가 없었던 것 같아 빈 주입을 통하도록 변경하였습니다. 😁
@Primary | ||
@DependsOn({ROUTING_DATASOURCE}) | ||
public DataSource dataSource() { | ||
return new LazyConnectionDataSourceProxy(routingDataSource()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
학습 차원의 질문이에요! LazyConnectionDataSourceProxy
는 어떻게 동작할까요? 여기서 필요한 이유는 무엇일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LazyConnectionDataSourceProxy란 데이터 소스에 대한 프록시 객체로, 첫번째 Statement을 생성하기 전까지 실제 JDBC Connections을 느리게 가져오는 객체입니다.
느리게 가져오더라도 auto-commit 모드, 트랜잭션 격리 및 read-only 모드와 같은 Connection 초기화 속성은 유지되며 실제 연결이 가져오는 즉시 실제 JDBC 연결에 적용됩니다.
예를 들어 아래와 같은 상황에서 LazyConnectionDataSourceProxy를 통하게 되면 Statement 생성 시점 전까지는 실제 커넥션이 아닌, ProxyConnection을 가져오게 되는데요.
// 예시
@Autowired
private DataSource dataSource;
public void proxyDatasource() {
// 이 시점에서는 실제 DB 연결이 발생하지 않음
Connection conn = dataSource.getConnection();
// 실제 Statement 생성 시점에 DB 연결이 발생
Statement stmt = conn.createStatement();
}
LazyConnectionDataSourceProxy 를 등록해주어야 하는 이유는 이 데이터 소스 프록시를 사용하면 실제로 필요한 경우가 아니면 풀에서 JDBC 연결을 가져오는 것을 피할 수 있기 때문입니다.
즉, 연결을 했는데 실제로 Statement 문을 사용하지 않는 경우에는 비효율적으로 Connection을 차지하고 있게 되기 때문에 불필요한 리소스 점유를 예방할 수 있게 됩니다. 트랜잭션 관리는 하지만, 실제 DB 연결은 필요한 시점까지 지연시킬 수 있어 리소스를 효율적으로 사용할 수 있다고 볼 수 있습니다.
this.name = new Name(name); | ||
this.discountAmount = new DiscountAmount(discountAmount); | ||
this.discountRate = DiscountRate.from(discountAmount, minimumOrderAmount); | ||
this.minimumOrderAmount = new MinimumOrderAmount(minimumOrderAmount); | ||
this.issuancePeriod = new IssuancePeriod(issueStartDate, issueEndDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 파라미터가 참조형 타입인데요. null은 검증하지 않아도 괜찮을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
검증 추가하였습니다!
public record DiscountRate(Integer discountRate) { | ||
|
||
private static final double MINIMUM_DISCOUNT_RATE = 3; | ||
private static final double MAXIMUM_DISCOUNT_RATE = 20; | ||
|
||
public DiscountRate { | ||
validateRateRange(discountRate); | ||
} | ||
|
||
public static DiscountRate from(Long discountAmount, Integer minimumOrderAmount) { | ||
return new DiscountRate((int) ((double) discountAmount / minimumOrderAmount * 100)); | ||
} | ||
|
||
public void validateRateRange(final Integer discountRate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
integer, long, double이 혼재되어 있는데요.
한 가지 타입만 사용해도 괜찮지 않을까요? 몰리는 어떻게 생각하나요~?
프로젝트 전체적으로도 고려해본다면 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long을 사용한 것은 금액이라는 도메인을 고려하였고, Integer를 사용한 것은 할인률과 소수점을 버림하는 요구사항을 고려하였는데요.
다시보니 double은 불필요하고 도메인 요구사항의 의미가 혼동되어 보일 수 있을 것 같네요.
Long과 Integer만 사용되어도 좋을 것 같아 변경하겠습니다!
|
||
@Column(name = "discount_rate", nullable = false) | ||
@Embedded | ||
private DiscountRate discountRate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discountRate
는 discountAmount
와 minimumOrderPrice
에 의해 결정되죠.
필드로 선언하지 않아도 기능은 충분할 것 같은데, 필드로 선언하면 어떤 장점이 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필드로 선언하면 어떤 장점이 있을까요?
객체를 생성하는 비용이 없을 수는 있지만, 사실 저는 그 장점을 크게 느끼지 못한 것 같아요. 🥲
현재 요구사항에 주어진 할인율에 대한 책임이 도메인 객체로 분리하기 충분하다고 생각했기 때문인데요.
할인율 제약
할인율은 할인금액 / 최소 주문 금액 식을 사용하여 계산하며, 소수점은 버림 한다.
예를 들어, 최소 주문 금액이 30,000원일 때 할인 금액이 1,000원이라면 쿠폰의 할인율은 3%(1000 / 30000 = 3.333...%)가 된다.
할인율은 3% 이상이어야 한다.
할인율은 20% 이하여야 한다.
반대로 분리하였을 때의 장점은 관련 책임을 Coupon
이 가지고 있는 것이 아닌 DiscountRate
가 가지게 됩니다. 객체를 분리하고 가장 적절한 객체에서 독립적인 책임을 부여하는 선택을 했던 것 같아요 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
데모데이 정말 고생 많았어요 몰리!!
1단계 요구사항은 모두 반영하셨기 때문에, 남은 이야기는 2단계에서 이어가보도록 해요.
주말도 정말 바쁘겠네요. 화이팅입니다!!! 💪💪💪💪💪💪💪💪💪💪💪💪💪💪💪💪💪
안녕하세요 제우스
서로 매일 리뷰를 하는 것 같은데
(ㅋㅋ)미션에서는 처음이네요 😁이번 복제 지연 미션에 대해 고민해본 점은 아래와 같아요.
복제 지연 발생 시 해결 방법
1. Write 쿼리 발생 후 일정 시간동안 조회 쿼리를 Write DB로부터 가져온다.
Read DB가 동기화된 이후에 다시 Read DB를 바라보게 하여 조회 쿼리를 실행한다.
2. 별도의 cache를 통해 업데이트 발생 시 일정 시간동안 cache를 읽도록 한다.
캐시와 같은 별도의 인메모리 저장소에 Write된 데이터를 저장하고, 조회 쿼리를 실행할 때 캐시에 저장된 데이터를 조회한다.
3. Write 쿼리 발생 후 복제가 완료될 때까지 대기한다.
sleep 을 주어 Read DB까지 복제 완료되기까지 대기하도록 한다.
캐시를 사용해보고 싶다는 마음이 있었지만, 현재 상황에서 캐시 사용은 오버 엔지니어링이라고 느껴졌어요.
(2단계 캐시인 걸 알고 있기도 하구요 ㅋㅋㅋ)따라서 1번 방식으로 진행했습니다!
이번 미션도 같이 재미있게 의견 나누면 좋을 것 같아요. 잘 부탁드립니다. 파이팅 🔥