-
Notifications
You must be signed in to change notification settings - Fork 4
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
db replication, 라우팅 추가 #454
Conversation
여러분 이거 merge 했다고 원래 있던 커밋 다끌고 오는게 맞나요 ?; 원래 안그랬던 것 같은데 ............ |
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.
코드레벨에서 변경사항은 확실히 적네요!
오션의 레플리케이션 세미나 기대하겠습니다!
그리고 궁금한 점이 있는데,
master, slave에 따라 권한도 변경해야 하나요??
protected Object determineCurrentLookupKey() { | ||
boolean isReadOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly(); | ||
|
||
return isReadOnly ? SLAVE_DATA_SOURCE : MASTER_DATA_SOURCE; |
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.
아닠ㅋㅋㅋㅋㅋ 도치 PR에 else 가지고 뭐라하더니 ㅋㅋㅋㅋ
그치만 전 삼항연산자 좋아합니다.
이정도는 써도 된다고 생각하는 편이에요
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.
ㅎㅎ.. 다시 원복했습니다
spring.datasource.master.jdbc-url=jdbc:mysql://localhost:20000/yozm-cafe?useSSL=false&allowPublicKeyRetrieval=true&characterEncoding=UTF-8&serverTimezone=UTC | ||
spring.datasource.master.username=root | ||
spring.datasource.master.password=root | ||
spring.datasource.master.driver-class-name=com.mysql.cj.jdbc.Driver | ||
|
||
spring.datasource.slave.jdbc-url=jdbc:mysql://localhost:20001/yozm-cafe?useSSL=false&allowPublicKeyRetrieval=true&characterEncoding=UTF-8&serverTimezone=UTC | ||
spring.datasource.slave.username=root | ||
spring.datasource.slave.password=root | ||
spring.datasource.slave.driver-class-name=com.mysql.cj.jdbc.Driver |
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.
확실히 오션말대로 application.properties 가독성이 점점 사망하고 있는게 느껴지네요..
다음에 yml 로 변경하던지 해야겠어요..!
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.
이번 pr에서 yml 파일로 변경할 예정입니다!
public static final String MASTER_DATA_SOURCE = "master"; | ||
public static final String SLAVE_DATA_SOURCE = "slave"; |
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.
dataSourceConfig 에도 똑같은 값이 있네요. 두군데중 하나만 선언하시려고 public으로 하신 것 같은데, 까먹으신거 같네요 ㅎㅎ
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.
고생하셨어요 오션~
덕분에 routingDataSource로 datasource를 제어할 수도 있다는 걸 알았네요
@Bean | ||
@Primary | ||
public DataSource dataSource(@Qualifier(ROUTING_DATA_SOURCE) DataSource routingDataSource) { | ||
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.
오 이건 왜 하나 했더니 트랜잭션에 진입하자마자 connection을 결정하지 않고 유보하려고 한 의도인가요? 이런 것도 있었군요.
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.
Spring이 트랜잭션에 진입하는 순간 이미 설정된 DataSource의 커넥션을 가져오고 트랜잭션을 식별하면 DataSource의 커넥션을 가져옵니다. 이럴 경우 지금처럼 master/slave DataSource 환경에서는 DataSource를 선택하는 분기가 불가능하기 때문에 미리 DataSource를 정하지 않도록 LazyConnectionDataSourceProxy
를 사용하여 실제 쿼리가 실행될 때 Connection을 가져오도록 설정하는 로직입니다!
|
||
@Override | ||
protected Object determineCurrentLookupKey() { | ||
boolean isReadOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly(); |
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.
final?
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.
good
protected Object determineCurrentLookupKey() { | ||
boolean isReadOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly(); | ||
|
||
return isReadOnly ? SLAVE_DATA_SOURCE : MASTER_DATA_SOURCE; |
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.
이것 때문에 상수를 Enum으로 관리하려 하셨군요!
흠 저는 DataSourceConfig쪽에서 public으로 열어놓고 이쪽에서 받아서 사용하는 게 어떨까합니다.
public으로 열어놓는 게 마음에 들지 않는다면 private으로 닫아놓고 DataSourceConfig에 static한 getter를 만들어 쓸 수도 있을 것 같아요
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.
고생하셨습니다!
DB Replication은 신경써줘야 할 것이 많네요...ㄷㄷ
@@ -14,6 +14,7 @@ services: | |||
command: [ "mysqld", "--character-set-server=utf8mb4", "--collation-server=utf8mb4_general_ci" ] | |||
volumes: | |||
- db-data:/var/lib/mysql | |||
- ./my.cnf:/etc/my.cnf |
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.
주석 추가하겠습니다~
public static final String MASTER_DATA_SOURCE = "master"; | ||
public static final String SLAVE_DATA_SOURCE = "slave"; | ||
private static final String ROUTING_DATA_SOURCE = "routingDataSource"; | ||
private static final String MASTER_DATA_SOURCE_LOCATION = "spring.datasource.master"; | ||
private static final String SLAVE_DATA_SOURCE_LOCATION = "spring.datasource.slave"; | ||
|
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.
음 .. 전 뭔가 타입(location, source 등) 별로 나뉘는게 좀 더 가독성이 좋다고 생각했는데 반영해보겠습니다!
public static final String MASTER_DATA_SOURCE = "master"; | ||
public static final String SLAVE_DATA_SOURCE = "slave"; |
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.
개인적인 생각으로는 DataSourceConfig
의 상수를 public하게 쓰는 것이 좋을 것 같아요!
boolean isReadOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly(); | ||
|
||
return isReadOnly ? SLAVE_DATA_SOURCE : MASTER_DATA_SOURCE; |
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.
boolean isReadOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly(); | |
return isReadOnly ? SLAVE_DATA_SOURCE : MASTER_DATA_SOURCE; | |
if(isCurrentTransactionReadOnly()){ | |
return SLAVE_DATA_SOURCE; | |
} | |
return MASTER_DATA_SOURCE; |
static import와 함께 이건 어떤가요...?
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.
ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ ㅠㅠ 수정했슴다
|
fdee2d2
to
347e3e3
Compare
#️⃣ 연관된 이슈
📝 작업 내용
더 자세한 것은 주말내로 블로깅을 할 예정인데, 간단하게 말하면
routingDataSource
메소드에서 삽입해줍니다. 그러면 datasource등록이 끝납니다.determineCurrentLookupKey
에서 판단하여 빈으로 등록하여 datasource등록한 master/slave를 찾아 이를 사용하는 방식입니다.큰 걱정
replication을 한다면 db 복제 -> master/slave 연결 과정을 거쳐야 합니다. 다만 db 복제 와 연결 과정 사이에 새로운 db가 들어간다면 replicate db, master db 간 정합도가 깨질 수도 있습니다. 이 과정을 어떻게 해결하면 좋을지 걱정되네요 흠..
💬 리뷰 요구사항