Skip to content

Conversation

@ZZAMBAs
Copy link

@ZZAMBAs ZZAMBAs commented Jan 28, 2025

아직 테스트 코드 작성을 하지 못했습니다.

전체적 구성

image

  • DB에 User, Account 테이블을 생성했고 각각은 사용자, 계좌 정보를 뜻합니다. 사용자 정보는 간단히 구현했고 따로 인증을 하지는 않습니다.
  • Account 테이블에는 적금 계좌, 입출금 계좌 전부 들어갈 수 있고 account_type 컬럼으로 이를 판별합니다. (CHECKING: 입출금, INSTALLATION: 적금). 계좌번호에 unique 인덱스를 설정했습니다.
  • 인당 출전 한도는 User 테이블에서 관리합니다. 관련 컬럼은 day_withdraw_limit 입니다. 이에 따라 해당 사용자의 일일 출전 금액과 마지막 출전 일시를 저장할 필요가 있다 생각해서 day_withdraw 컬럼과 last_withdraw_date 컬럼을 추가했습니다.
  • 적금 계좌는 자유 적금 계좌로 가정했습니다. 이에 따라 따로 특정 일시마다 돈이 빠져나가는 시스템은 아닙니다.

구현사항

각 구현 사항에 대해 제 구현은 다음과 같습니다.

  • 각 사용자가 여러 계좌를 생성할 수 있게 만들어야 합니다.
    • 계좌 생성 API로 수행 가능합니다. 사용자 Id 값과 입출금/적금 계좌 종류를 인자로 받아 생성할 수 있습니다. (아직 메인 계좌 변경 API는 없음)
  • 메인 계좌는 외부 계좌에서 돈을 가져오는 기능이 주 기능이므로, 금액 추가가 가능합니다. 1일 출전 한도는 3,000,000원 입니다.
    • 출전 한도는 User 테이블에서 관리하며 거래(출금, 송금) 시, 그 출금이 한도를 초과하는지 먼저 검사한 후, 처리가 됩니다.
  • 적금 계좌는 메인 계좌에서 돈을 인출할 수 있으며, 메인 계좌의 돈이 없으면 인출할 수 없습니다.
    • 송금 시, 송금하는 계좌가 적금 계좌에 돈을 넣을 수 있는 계좌인지 판별한 후, 가능하면 거래합니다.

프로그래밍 요구사항

  • 적금 계좌 - 메인 계좌 간 트랜잭션
    • Read Uncommitted는 트랜잭션 간 간섭이 존재할 수 있기에 제외, Serializable은 MySQL에서 단순 조회문도 락을 걸기에 제외했습니다. Repeatable Read는 DML 처리를 할 때, 확인하는 모든 레코드에 X락을 겁니다. 그러나 Read Committed는 WHERE 절 처리에 속하지 않는 레코드는 락을 바로 풀게 됩니다. 데드락 가능성을 줄여줄 수 있다고 생각하였고, SELECT 시에도 Repeatable Read는 스냅샷을 가져오는 반면, Read Committed는 최신 커밋을 반영한 데이터를 스냅샷으로 갱신하고 가져옵니다. 최신 데이터가 중요한 시스템이라 생각하여 Read Committed를 선택했습니다.
    • A 계좌, B 계좌가 있을 때, A->B, B->A로 동시 송금이 발생하는 경우, 상황에 따라 두 계좌가 모두 X락이 걸려 데드락이 발생하는 상황을 발견했습니다. 이에 따라 계좌 번호 사전순 처리를 통해 데드락을 해결했습니다.
  • 인당 한도 관리
    • 컬럼으로 일단 관리를 하는데 일마다 초기화 하는 부분을 생각해 보았습니다. 테이블을 초기화하는 것은 느리고, 캐시로 두자니 트랜잭션 실패 처리가 힘들 것 같았습니다. 이에 따라 컬럼으로 따로 두고 지연 초기화하기로 했습니다. User 테이블 내 last_withdraw_date를 참고하고 거래 일시와 비교해 하루가 지났으면 해당 테이블 내 day_withdraw를 0으로 초기화합니다. 그렇지 않으면 그대로 둡니다.

문제점

아직 다음 문제가 존재합니다.

  • 락으로 인한 지연
    • A 계좌와 B 계좌가 있을 때, A->B 거래(ㄱ) 도중 다른 C->A 거래가 일어나거나 B->C 거래가 일어날 때, (ㄱ)의 처리가 느려지면 락을 대기하느라 같이 느려지는 현상이 있습니다. 해결책을 찾는 중입니다.

@ZZAMBAs ZZAMBAs requested a review from VSFe January 28, 2025 05:38
@ZZAMBAs ZZAMBAs self-assigned this Jan 28, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
9.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link

@opixxx opixxx left a comment

Choose a reason for hiding this comment

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

수고하셨습니다.~

@@ -0,0 +1,4 @@
package org.c4marathon.assignment.domain.dto.request;

public record TransferRequest(String senderAccountNumber, String receiverAccountNumber, long money) {
Copy link

Choose a reason for hiding this comment

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

AccountNumber에 공백이 포함되면 어떻게 되나요?

@@ -0,0 +1,4 @@
package org.c4marathon.assignment.domain.dto.request;

public record WithdrawRequest(String accountNumber, long money) {
Copy link

Choose a reason for hiding this comment

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

money가 음수일 경우 동작이 반대로 될 것 같습니다. 전반적인 입력값 유효성 검증이 필요할 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

프로그래밍 요구사항만 생각하다보니 기본 유효성 검사와 에러 처리 등을 하지 않은 것 같아요. 시간 날 때 말씀하신대로 처리해 보겠습니다!

import lombok.NoArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Copy link

Choose a reason for hiding this comment

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

log를 사용안해서 굳이 필요하지 않아보입니다.

Copy link
Author

Choose a reason for hiding this comment

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

추후에 지우겠습니다

public interface AccountRepository extends JpaRepository<Account, Long> {
@Modifying
@Query("UPDATE Account a SET a.balance = a.balance + :money WHERE a.accountNumber = :accountNumber AND a.balance + :money >= 0")
int addBalance(@Param("money") long money, @Param("accountNumber") String accountNumber);
Copy link

Choose a reason for hiding this comment

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

다른 곳에선 @param을 사용안해서 통일하면 좋아보여요

Copy link
Author

Choose a reason for hiding this comment

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

스프링 데이터 JPA에서 @param을 안쓰면 에러가 나기도 한다는 것 같아서 통일하겠습니다!

https://zzang9ha.tistory.com/357

*/
@Transactional(isolation = Isolation.READ_COMMITTED)
public CreatedAccountInfo create(long userId, AccountType accountType) {
userRepository.findById(userId).orElseThrow(() -> new RuntimeException("User not found."));
Copy link

Choose a reason for hiding this comment

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

나중에 예외처리도 의미 있게 처리하면 좋을 것 같아요

@ZZAMBAs ZZAMBAs requested a review from hellozo0 January 28, 2025 14:59
Copy link
Contributor

@hellozo0 hellozo0 left a comment

Choose a reason for hiding this comment

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

일일 한도를 User에 넣어서 부하를 줄이는 방법은 미처 생각해보지 못했는데 좋은 아이디어 중에 하나 인것 같습니다! 현재 step1에 조건 사항에는 맞는 방법이라 생각이 드네요

구현 방식 또한 다른 접근 방식에서 생각해볼 수 있어서 좋았습니다. 저는 최신 데이터보다는 정합성이 중요하다고 생각해서 읽기 부터 XLOCK으로 데이터를 잠그고 가져오는 방식을 생각했는데 다음 스텝때는 참고해서 개선할 수 있는것은 개선해보겠습니다! 수고하셨습니다 :)

@ZZAMBAs ZZAMBAs merged commit cdea504 into base/zzambas Feb 23, 2025
2 of 3 checks passed
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.

4 participants