Skip to content
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

RDS reader writer 적용 #701

Merged
merged 5 commits into from
Sep 26, 2024
Merged

RDS reader writer 적용 #701

merged 5 commits into from
Sep 26, 2024

Conversation

HoeSeong123
Copy link
Contributor

⚡️ 관련 이슈

close #699

📍주요 변경 사항

  1. 운영 환경의 데이터베이스를 RDS로 변경합니다.
  2. @Transational(readOnly=true)를 설정하면 Reader 인스턴스를 사용하게 되고, @Transactional을 사용하거나 아예 사용하지 않으면 Writer 인스턴스를 사용합니다.
  3. 해당 설정은 운영환경에만 적용됩니다.

@HoeSeong123 HoeSeong123 added refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항 BE 백엔드 labels Sep 26, 2024
@HoeSeong123 HoeSeong123 added this to the 5차 스프린트🍗 milestone Sep 26, 2024
@HoeSeong123 HoeSeong123 self-assigned this Sep 26, 2024
Copy link
Contributor

@zeus6768 zeus6768 left a comment

Choose a reason for hiding this comment

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

초롱짱

궁금한거 물어봤어용

Comment on lines +15 to +16
<root level="INFO">
<appender-ref ref="info-appender"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 부분은 앞으로도 계속 적용할 정책인가요? 아니면 데모데이 이후 변경될 예정인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 설정 파일이 잘 적용되는지 정도만 확인하고 삭제하겠습니다~

jminkkk
jminkkk previously approved these changes Sep 26, 2024
Copy link
Contributor

@jminkkk jminkkk left a comment

Choose a reason for hiding this comment

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

초롱짱
저도 궁금한거 살짝쿵 적어봤어요 깔깔


@Override
protected Object determineCurrentLookupKey() {
boolean readOnly = TransactionSynchronizationManager.isCurrentTransactionReadOnly();
Copy link
Contributor

Choose a reason for hiding this comment

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

새로운 트랜잭션인 경우, TransactionSynchronizationManager 에 새로운 트랜잭션을 등록합니다.

TransactionSynchronizationManager으로부터 지금 실행되는 트랜잭션이 readOnly으로 마킹되어있는지 체크하는가 보군요 신기방기

Comment on lines +29 to +32
@ConfigurationProperties("spring.datasource.write")
public DataSource writeDataSource() {
return DataSourceBuilder.create().type(HikariDataSource.class).build();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

따로 "spring.datasource.write" 값을 주입하는 코드가 없는데 자동으로 주입이 되는 건가요? 이것도 신기방기하네요

Copy link
Contributor

Choose a reason for hiding this comment

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

ㄹㅇㄹㅇ 신기한게 한두개가 아님요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아마 @ConfigurationProperties를 리플렉션해서 주입해주는 코드가 내부에 있을 것 같네요

Copy link
Contributor

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

reader / writer 라우팅을 어플리케이션 코드로 할 수 있다는게 진짜 신기하네용
사소한 궁금증 + 제안 하나 남겨두었으니 확인 부탁드려요!

DataSource readerDataSource = readeDataSource();

Map<Object, Object> dataSourceMap = new HashMap<>();
dataSourceMap.put("write", writerDataSource);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dataSourceMap.put("write", writerDataSource);
dataSourceMap.put(DataSourceRouter.WRITER_LOOKUP_KEY, writerDataSource);

위의 키들은 DataSourceRouter 의 반환값과 맞춰 주어야 하는건가요??
그렇다면 오탈자 때문에 오류가 나는 것을 예방하기 위해 상수로 관리하는 것은 어떤가요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은데요???!

Copy link
Contributor

@kyum-q kyum-q left a comment

Choose a reason for hiding this comment

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

초롱 짱 ~~ 신기방기네요 ~
Transactional 관련 코멘트 남겨뒀어요 확인부탁드려요 ~

@@ -52,6 +52,7 @@ public Page<Template> findAllBy(
return templateRepository.findAll(new TemplateSpecification(memberId, keyword, categoryId, tagIds), pageable);
}

@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

위에 create에도 @transactional 해주세요 ~

jminkkk
jminkkk previously approved these changes Sep 26, 2024
kyum-q
kyum-q previously approved these changes Sep 26, 2024
@HoeSeong123 HoeSeong123 dismissed stale reviews from kyum-q and jminkkk via 55684ff September 26, 2024 13:58
Copy link
Contributor

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

👍

@kyum-q kyum-q merged commit 9c3c12b into dev/be Sep 26, 2024
6 checks passed
@kyum-q kyum-q deleted the refactor/699-reader-writer-rds branch September 26, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

5 participants