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

[test] uploadPhoto, updateEmail 동시 호출 시 문제 확인 및 테스트 #46

Closed
wants to merge 17 commits into from

Conversation

RumosZin
Copy link
Contributor

@RumosZin RumosZin commented Sep 5, 2024

연관 이슈

작업 내용

  • uploadPhoto, updateEmail 두 트랜잭션을 동시에 호출하는 테스트를 진행했습니다.
  • 동시 호출 시 "두 번의 갱신 분실 문제"가 발생했습니다. photo_request를 수정하는 두 트랜잭션 중 나중 커밋 내용만 남고 이전 커밋 내용은 사라지는 문제입니다. 이로 인해 사용자에게는 이메일이 정상적으로 변경되었다고 알람이 가지만, 이미지는 옛날 이메일로 전송되는 문제가 발생할 수 있어서 꼭 해결해야 하는 문제입니다.
  • 낙관적 락, 비관적 락으로 문제를 해결해보았습니다.

문제 파악 / 해결 방식에 대해 PR에 자세히 적긴 어려울 것 같아 따로 작성하고 링크를 남깁니다.

리뷰 요구사항

두 가지 방식으로 해결이 가능하나, 하나를 골라서 적용해야 한다면 낙관적 락이 나은 방식이라고 생각합니다.

비관적 락을 사용하면 간단히 데이터의 일관성을 보장하지만, 순차적으로 트랜잭션을 수행하므로 성능이 저하될 우려가 있습니다.

이에 비해 낙관적 락을 사용하면 재시도 로직을 구현해야하지만 락에 의한 조회 시 성능 저하는 발생하지 않습니다. 한 메서드에 여러 요청이 몰려서 충돌이 잦을 것으로 예상되면 비관적 락을 골랐을 것이지만, 현재는 두 메서드가 만에 하나 같은 데이터를 수정하려 하는 경우 발생하는 문제이므로, 낙관적 락을 이용한 방식이 나을 것 같습니다.

낙관적 락으로 해결 3c69103 < 누르면 comment 있습니당
비관적 락으로 해결 4210ea3

각각 코드 보시고 의견 주세요!

@RumosZin RumosZin added refactor 성능 개선 및 코드 리팩토링 test 테스트코드 labels Sep 5, 2024
@RumosZin RumosZin self-assigned this Sep 5, 2024
Copy link
Contributor

@win-luck win-luck left a comment

Choose a reason for hiding this comment

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

CS 지식을 바탕으로 의미 있는 실험 및 검증이었다고 생각합니다. 고생하셨습니다~!

Comment on lines +64 to +65
System.out.println("success count: " + successCount.get());
System.out.println("fail count: " + failCount.get());
Copy link
Contributor

Choose a reason for hiding this comment

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

System.out보다는 log를 활용하시는 게 더 적합할 것 같습니다.

Comment on lines +22 to +23
@Test
void 수정_조회가_동시에_발생하는_경우_테스트() throws InterruptedException {
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트명을 한글로 작성하셔도 되지만 @DisplayName 어노테이션으로 별도로 분리하시는 것도 좋습니다.

Copy link
Contributor

@alsrudrl1220 alsrudrl1220 left a comment

Choose a reason for hiding this comment

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

사용자가 이미지 생성을 기다리는 중에 이메일을 변경하려는 상황을 가정하고, 두 메서드가 동시에 실행되는 경우 수정사항이 분실되는 것을 해결하기 위한 의미있는 시도라고 생각합니다~!

제시하신 문제 상황이 빈번하게 일어나는 상황은 아니기 때문에, 비관적 락으로 인해 불필요하게 조회를 못하고 트랜잭션이 대기하는 일이 생기는 것보다 말씀해주신 것처럼 낙관적 락을 사용하여 성능 저하를 막는 것이 좋아보입니다.
낙관적 락의 구현 방식으로 @Version을 통해 "데이터를 수정할 때 변경이력을 확인"하여 @Retryable로 "트랜잭션 충돌 시 재시도하는" 방법 좋다고 생각합니다.

해당 이슈와 관련해서 이전에 이메일 전송의 트랜잭션 범위에 대해 의논을 했던 것과 같이,
메일 전송은 구글의 smtp를 사용하기 때문에 메일 서버와 통신할 수 없는 상황이 발생하는 것을 고려하여 "메일 전송 부분은 트랜잭션에 제외시켜서 스레드를 유지하지 않는 것"이 좋다는 것과도 연관이 있어서 좋은 주제라고 생각이 듭니다!

수고 많으셨습니다~

@RumosZin RumosZin closed this Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 성능 개선 및 코드 리팩토링 test 테스트코드
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[test] uploadPhoto, updateEmail 동시 호출 시 문제 확인 및 테스트
3 participants