Conversation
Summary of ChangesHello @JayongLee, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 Redisson 분산 락과 JPA 비관적 락을 통합하여 시스템의 데이터 정합성을 크게 향상시키는 것을 목표로 합니다. 특히 계좌 거래 및 해외 송금과 같은 중요한 금융 작업에서 발생할 수 있는 동시성 문제를 방지하여, 여러 사용자가 동시에 계좌에 접근할 때 데이터 불일치가 발생하지 않도록 보장합니다. 이를 통해 시스템의 안정성과 신뢰성을 강화합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
이 PR은 동시 계좌 거래 시 데이터 정합성을 향상시키기 위해 분산 락(Redisson)과 비관적 락(pessimistic lock)을 도입했습니다. 분산 락과 데이터베이스의 비관적 락을 함께 사용하는 접근 방식은 견고하며 좋은 시도입니다. 다만, 몇 가지 개선점을 발견했습니다. 특히 Redisson 락의 임대 시간(lease time) 설정에 데이터 정합성을 해칠 수 있는 심각한 문제가 있으며, 의존성 버전 문제로 빌드가 실패할 수 있습니다. 또한 일부 설정 개선의 여지가 있습니다. 자세한 내용은 아래 개별 코멘트를 확인해주세요.
I am having trouble creating individual review comments. Click here to see my feedback.
build.gradle (39)
Redisson 의존성 버전 3.45.1은 Maven Central에 존재하지 않는 것으로 보입니다. 아마도 3.25.1의 오타이거나 다른 버전을 의도하신 것 같습니다. 이로 인해 빌드가 실패할 수 있습니다. 프로젝트의 Spring Boot 버전에 맞는 최신 안정 버전(예: 3.29.0) 또는 프로젝트에서 사용하기로 한 정확한 버전으로 수정하는 것을 권장합니다.
implementation 'org.redisson:redisson-spring-boot-starter:3.29.0'
src/main/java/org/creditto/core_banking/domain/account/service/AccountLockService.java (17)
Redisson의 tryLock 메서드에 설정된 leaseTime (15초)이 만료되면, 락을 획득한 스레드의 작업이 끝나지 않았더라도 다른 스레드가 락을 획득할 수 있습니다. 이는 데이터 정합성을 깨뜨릴 수 있는 심각한 문제입니다. tryLock은 lock 메서드와 달리 lease time이 만료되어도 자동으로 연장해주는 Watchdog을 사용하지 않습니다.
데이터베이스 트랜잭션이나 다른 작업이 15초 이상 소요될 가능성이 있다면, leaseTime을 훨씬 더 긴 시간(예: 60초)으로 설정하거나, 이 값을 외부 설정으로 분리하여 환경에 맞게 조정할 수 있도록 하는 것이 안전합니다.
private static final long LOCK_LEASE_MILLIS = 60000L;
src/main/resources/application.yml (11-13)
Redis 연결 정보를 하드코딩하는 대신, 데이터베이스 설정처럼 환경 변수를 사용하도록 변경하는 것이 좋습니다. 이렇게 하면 개발, 스테이징, 프로덕션 등 다양한 환경에 배포할 때 유연하게 대처할 수 있습니다. 예를 들어, host: ${REDIS_HOST:localhost}와 같이 기본값을 지정하는 방식을 사용할 수 있습니다.
host: ${REDIS_HOST:localhost}
port: ${REDIS_PORT:6379}
timeout: ${REDIS_TIMEOUT:2000}|
/gemini review |
There was a problem hiding this comment.
Code Review
이번 PR은 동시성 환경에서 데이터 정합성을 향상시키기 위해 Redisson을 이용한 분산 락과 데이터베이스 비관적 락을 도입했습니다. AccountLockService를 통해 락 관련 로직을 분리하고 설정을 추가하는 등 코드 구조가 잘 잡혀있습니다. 동시성 테스트를 추가한 점도 매우 좋습니다. 다만, RemittanceProcessorService에서 외부 네트워크 호출을 포함하는 긴 트랜잭션을 발견했습니다. 이는 성능 저하의 원인이 될 수 있습니다. 관련하여 개선 방안에 대한 상세한 리뷰를 남겼으니 확인 부탁드립니다.
| @@ -67,7 +69,6 @@ public class RemittanceProcessorService { | |||
| @Transactional | |||
There was a problem hiding this comment.
현재 execute 메서드에 @Transactional이 적용되어 있어 메서드 시작 시점에 트랜잭션이 시작됩니다. 하지만 이 트랜잭션 내에서 exchangeService.exchange()(88라인)를 통해 외부 네트워크 호출이 발생하고 있습니다.
네트워크 호출과 같이 오래 걸릴 수 있는 작업을 트랜잭션 내에서 수행하는 것은 다음과 같은 이유로 안티패턴으로 간주됩니다.
- 메서드가 실행되는 동안 DB 커넥션 풀의 커넥션을 계속 점유하여, 높은 부하 상황에서 커넥션 풀 고갈을 유발할 수 있습니다.
- 트랜잭션이 잠금을 사용하는 경우, 잠금 유지 시간이 길어져 경합이 증가하고 데드락 발생 가능성이 높아집니다.
트랜잭션의 범위를 최소화하도록 리팩토링하는 것을 권장합니다. 트랜잭션은 잔액 확인, 계좌 업데이트, 송금 및 거래 내역 저장 등 원자적으로 실행되어야 하는 최종 작업만 감싸야 합니다.
아래와 같은 리팩토링을 제안합니다.
execute메서드에서@Transactional애노테이션을 제거합니다.processRemittanceTransaction과 같은 새로운 private 또는 protected 메서드를 만들고@Transactional을 적용합니다.- 원자성이 필요한 로직을 이 새로운 메서드로 옮깁니다.
execute메서드에서 외부 호출이 완료된 후 이 새로운 트랜잭션 메서드를 호출합니다.
// @Transactional 제거
public OverseasRemittanceResponseDto execute(final ExecuteRemittanceCommand command) {
// ... (엔티티 조회, 환전, 수수료 계산 등)
// 이 부분은 트랜잭션 없이 실행
// ...
// 원자성이 필요한 로직은 별도의 트랜잭션 메서드로 분리하여 호출
return processRemittanceTransaction(command, totalDeduction, recipient, regularRemittance, savedExchange, feeRecord, actualSendAmount);
}
@Transactional
protected OverseasRemittanceResponseDto processRemittanceTransaction(...) {
return accountLockService.executeWithLock(command.accountId(), () -> {
// 분산 락 및 비관적 락 내부 로직
// (잔액 확인, 출금, 송금 및 거래 내역 저장)
// ...
});
}이렇게 변경하면 데이터베이스 리소스를 더 빨리 해제하여 애플리케이션의 성능과 안정성을 향상시킬 수 있습니다.
🗞️ 연관된 이슈
🔥 이슈번호
✅ 작업 내용
📸 스크린샷 (선택)
체크리스트 ✅
기타