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

jdbc step4 주드 미션 제출합니다 #552

Merged
merged 3 commits into from
Oct 13, 2023

Conversation

kevstevie
Copy link

안녕하세요 푸우
4단계 정말 쉽지 않네요??
템플릿 콜백으로 중복 코드를 제거하고 app userservice와 txuserservice를 나눠서 비즈니스 - db 로직을 구분하는 것까진 수월했지만..
Required 형태의 트랜잭션 전파를 구현해야한다는 점이 어려워서 다른 코드를 좀 많이 참고 했습니다
특히 DataSourceUtils의 releaseConnection의 분기문들..

리뷰 잘부탁드립니다

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 주드!!
4단계 미션 정말 잘 구현해 주셨군요 👍👍👍👍 💯💯💯💯
구현해 주신 객체들을 보니 트랜잭션 메커니즘을 정말 잘 이해하고 있다는 것이 느껴졌습니다 ㅎㅎ
잘 구현해 주셔서 리뷰 드릴 부분이 크게 없었으나 몇 가지 개선할 만한 사항들이 보여 리뷰 남겼습니다!!

Comment on lines +14 to +19
public void createConnection() {
try {
Connection connection = dataSource.getConnection();
ConnectionHolder connectionHolder = new ConnectionHolder(connection);
connection.setAutoCommit(false);
connectionHolder.setIsTransactionActive(true);
Copy link

Choose a reason for hiding this comment

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

하나의 스레드 내에서 여러 트랜잭션이 있을 수 있으니
트랜잭션 매니저가 반환 값을 가지고, commit 과 rollback 에 해당 반환 값을 전달해주는 것은 어떨까요??

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신거 처럼 바꿔보려고 했는데 쉽지 않네요
TransactionTemplate 에서 롤백 시에 connection을 넘겨주는게 로컬변수 스코프가 달라서 불가능해요!

Comment on lines +79 to +80
} finally {
DataSourceUtils.releaseConnection(conn, dataSource);
Copy link

Choose a reason for hiding this comment

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

  1. transaction이 있을 경우 -> 아직 connection 을 닫지 않는다.
  2. transaction이 없을 경우 -> 다른 곳에서 connection 을 닫지 않으니 닫아준다.

위 두 가지 요구사항을 만족시키기 위해서 ConnectionHolder 가 정의 되었군요!!👍

Comment on lines 19 to 21
} catch (RuntimeException ex) {
transactionManager.rollback();
throw ex;
Copy link

Choose a reason for hiding this comment

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

RuntimeException 일 경우에만 rollback 하는 방식을 채택하셨군요!!
스프링의 트랜잭션의 경우 EJB의 관습을 따라 checked Exception 을 rollback 하지 않지만 TransactionTemplate 의 경우에는 모든 예외를 rollback 하네요!!
image

Comment on lines 15 to 20
try {
connection.commit();
} catch (Exception e) {
throw new RuntimeException(e);
}
}
Copy link

Choose a reason for hiding this comment

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

connectionHolder 의 트랜잭션 상태를 맞춰주기 위해서 setsetIsTransactionActive가 정의되었네요!!
하지만 이 setter 로 인해 connection 의 트랜잭션 정합성을 맞추기가 어려워진 것 같네요 🤔
commit과 rollback 에서 isTransactionActive 상태를 변경시켜주는 것이 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

커밋과 롤백이 일어날 경우 커넥션이 릴리즈되고 닫히기 때문에 상관없을 것 같습니다

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 주드!!
4단계 미션 정말 잘 구현해 주셨군요 👍👍👍👍 💯💯💯💯
구현해 주신 객체들을 보니 트랜잭션 메커니즘을 정말 잘 이해하고 있다는 것이 느껴졌습니다 ㅎㅎ
잘 구현해 주셔서 리뷰 드릴 부분이 크게 없었으나 몇 가지 개선할 만한 사항들이 보여 리뷰 남겼습니다!!

@kevstevie
Copy link
Author

안녕하세요 푸우
사실 이번 요청은 큰 변경사항이 없습니다
transaction exception에 대해서만 롤백을 하도록 수정했습니다 감사합니다

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 주드~!
미션 너무 수고하셨습니다 ㅎㅎ
다음 미션도 화이팅입니다!!

@BGuga BGuga merged commit 393ace4 into woowacourse:kevstevie Oct 13, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants