Feature/zzambas step2 #23
Conversation
- 기존 AccountService에서 가독성과 유지보수를 위해 분리
| * 유저 인당 한도 초기화 벌크 연산 | ||
| */ | ||
| @Scheduled(cron = "${user.charge-limit-initialization.execution-time}") | ||
| public void initChargeLimit() { |
There was a problem hiding this comment.
@Modifying을 사용해서 벌크 연산을 처리할 때 영속성 컨텍스트 1차 캐시를 고려해봐야할 것 같습니다.
There was a problem hiding this comment.
검색해보니 영속성 컨텍스트를 거치지 않고 쿼리를 처리하다보니 현재 영속성 컨텍스트와 값이 일치하지 않는 경우가 생길 수가 있군요.
https://wildeveloperetrain.tistory.com/142
@Modifying 옵션으로 영속성 컨텍스트를 지우거나 flush 할수도 있지만 영속성 컨텍스트 내부 값이 많다면 flush에도 시간이 소요될 것 같고 지우는 것도 다시 find해야 하는 부분으로 성능 손해가 있을 수도 있을 것 같습니다. 혹은 find 직후에 지워진다면 이것도 문제가 될 것 같네요.
일단 현재 저는 대부분의 업데이트 처리를 JPA 더티체킹 없이 직접 DB 쿼리로 하다보니 문제가 일어나지는 않을 것 같은데, 나중을 위해 고민해 봐야 할 것 같습니다. 혹시 픽스님은 이를 어떻게 처리하시나요?
| return ResponseEntity.status(HttpStatus.CREATED).body(accountService.create(userId, accountType)); | ||
| } | ||
|
|
||
| @PostMapping("/withdraw") |
There was a problem hiding this comment.
메인 계좌에서 돈을 출금하는 것으로 이해했는데, 맞나요?
Step1 외부계좌에서 메인 계좌로 충전하는 기능이 안보여서 질문드립니다.
There was a problem hiding this comment.
충전은 송금 메서드가 그 역할을 해줄 수 있다고 생각했습니다!
| * @return | ||
| * | ||
| * 계좌 생성. accountType은 생성 계좌가 적금 계좌인지 입출금 계좌인지를 판단. | ||
| */ |
There was a problem hiding this comment.
create에 @transactional을 사용하지 않은 이유가 있나요?
여러 사용자가 동시에 계좌 생성을 요청하면 문제가 안생기나요?
There was a problem hiding this comment.
읽기에는 따로 트랜잭션을 걸지 않아도 괜찮을 것이라 여겼고 그러면 트랜잭션을 걸어야 할 곳이 accountRepository.save() 한 곳인데 이미 SimpleJpaRepository에서 트랜잭션 처리를 하므로 따로 걸지는 않았습니다.
하지만 추후 테스트 코드로 다시 확인해 보겠습니다. 감사합니다.
| private final UserRepository userRepository; | ||
|
|
||
| // 단순 출금 | ||
| public WithdrawResult withdraw(String accountNumber, long money) { |
There was a problem hiding this comment.
혹시 여기서 트랜잭션을 시작안하고 updateBalance부터 트랜잭션을 시작한게 트랜잭션을 분리하기 위한 것인가요??
지금 코드에서는 withdraw에서부터 트랜잭션을 시작하는 것과 차이가 없다고 생각이 듭니다.
의견이 궁금합니다!
There was a problem hiding this comment.
말씀하신대로 여기서는 차이가 거의 없을 것 같아요. 따로 트랜잭션을 위한 클래스로 분리하다보니 AccountService에서는 모든 메서드에 @Transactional을 제거했습니다.
| * | ||
| * 적금 계좌는 송신 계좌가 메인 계좌여야 하기 때문에 그 조건을 판단하는 메서드. | ||
| */ | ||
| private void validateSender(String senderAccountNumber, String receiverAccountNumber) { |
There was a problem hiding this comment.
@transactional이 없어서 다른 트랜잭션에서 관련 Account를 변경하면 정합성 문제가 생기지 않을까요? 의견 궁금해요
There was a problem hiding this comment.
말씀하신대로 만약 관련 계좌를 찾아온 뒤에 DB내 수정이 발생하면 (적금->입출금 계좌로 전환 혹은 그 반대) 로직에 문제가 생길 수 있을 것 같아요. @Transactional 외 해결 방법도 있는지 찾아보겠습니다!
| .orElseThrow(() -> new RuntimeException("Account not found.")); | ||
|
|
||
| if (receiver.getAccountType() != AccountType.INSTALLATION) | ||
| return; |
There was a problem hiding this comment.
받는 계좌가 메인 계좌이면 더 이상 검증할 필요가 없어서 return 하는 것으로 이해했는데,
가독성을 고려하면 receiver.getAccountType() == AccountType.CHECKING이 코드를 더 이해하기 쉬웠던 것 같습니다. 개인적인 의견입니다
There was a problem hiding this comment.
말씀하신대로 가독성 측면에서는 ==이 더 좋을 것 같다 생각합니다. 그런데 저는 만약 다른 계좌 종류가 추가되어도 따로 제약 사항이 없다면 코드 수정 없이도 돌아갈 것이라 생각했었습니다.
| import java.util.Random; | ||
| import java.util.stream.IntStream; | ||
|
|
||
| public class AccountUtils { |
There was a problem hiding this comment.
AccountUtils도 CommonUtils처럼 인스턴스 생성 못하도록 하면 좋을 것 같습니다.
구현사항
프로그래밍 요구사항
송금 트랜잭션의 범위
송금 시에는 다음 과정이 일어납니다.
1-3 까지 과정을 전부 하나의 트랜잭션으로 묶기에는 너무 크다 여겼고, 이에 따라 각 과정을 트랜잭션으로 따로 나누었습니다. 이에 따라 락이 걸리는 시간이 줄어 데드락, 성능 하락 가능성을 줄였습니다.
동시 송금 처리
인당 한도 유효기간의 관리
application.yml파일로 관리해 동적 관리가 되도록 했습니다. 만약 인당 한도가 각자 부여되어야 한다면 컬럼으로 따로 추가할 계획입니다.아직 생각할 점
데드락 해결을 위해 현재 '사용자 내림차 순 확인' - '계좌 번호 내림차 순 확인' 순서로 트랜잭션을 처리하려고 하고 있지만 추후 조건이 더 늘어날 경우에 관리가 어려울 것 같은데 어떻게 해야 좋은 방안일지 고민 중에 있습니다.
테스트 코드는 현재 동시 송금 부분만 작성된 상태입니다.