[3주차 보완] redisson 분산락 도입#87
Merged
pyuseon merged 51 commits intohanghae-skillup:dbdb1114from Feb 4, 2025
Merged
Conversation
포크 레포지토리로 옮기면서 약간의 착오가 있어 다시 커밋합니다.
[ 2주차 완료 ] Feature/caching
[ 2주차 잔여 ] Feature/caching 캐싱 테스트 완료
This reverts commit c3163a8.
- init.sql : 변경된 칼럼명 반영 - application-dev , docke-compose : 컴포즈 실행 환경 수정 - redisConfig : @value 어노테이션 도입 - redis-repo/build.gradle : bootJar enabled=false 설정 추가
외부 서비스 연동을 위한 모듈
티켓 조회, 유저 티켓 조회, 티켓 예매 요청
AopForTransaction : 트랜잭션 분리를 위한 클래스 CustomSpringELParser : SPEL표현식 파서 DistributedLock : 분산락 어노테이션 정의 DistributedLockAOP : aop 분산락 작성
FunctionalForTransaction : 트랜잭션 분리를 위한 클래스 FunctionalDistributedLock : 함수형 분산락 구현
PEAK 타임을 고려한 최대 동시간대 요청 건수 계산 ---------------------------------------------------------- Junghyun's hanghaeho 인기영화 100편 비인기 영화 489편 모든 영화 1일 1회씩 상영, 2일 뒤 상영정보까지 전시 ---------------------------------------------------------- 인기 영화 100편의 경우 상영정보 오픈과 동시에 반 정도의 티켓이 바로 판매된다고 가정 100 * 13 = 1300건 비인기 영화 489편의 경우 상영정보 오픈과 동시에 약 3매 티켓 정도 바로 판매된다고 가정 489 * 3 = 1467건 피크시간 전체 판매 티켓 수 : 2767건 ---------------------------------------------------------- 예매는 한 사람이 여러좌석 예매 가능하므로 피크시간 예상 요청건수 = 922.33333 건 (전체 판매 티켓 수/3) 상영정보 조회 및 선택 - 좌석 선정 프로세스 소요시간 : 최대 30초 추정 ---------------------------------------------------------- 테스트상 30초까지 점진적으로 1000건의 요청시 최대 요청 처리시간 : 483.43ms - waitTime 동안 Lock을 기다릴 이유는 무엇인가? 먼저 좌석을 선정한 고객이 불가피한 이유로 결제 과정까지 넘어가지 못 했을 때 결제 과정까지 넘어가지 못하는 상황 1. 로그인, 연령 부적합 등 애초에 좌석 선정 자체가 불가한 경우 2. 결제 중 이탈 3. 단순 변심 4. ... 사실상 1번 말고는 기다렸다가 재시도 해야하고, 1번의 경우 매우 적은 수일 것으로 예상되어 실제로 Lock 점유를 기다리는 시간이 그다지 길 필요가 없다는 판단이 됩니다. 최악의 경우를 생각하여 앞의 요청 처리 두 건에서 lock점유 후 다음 프로세스로 진행이 못 했을 때 까지만 보장하기로 하여, wait_time은 1초, lease_time은 2초로 선정 lease_time 선정 이유는 만약 여러 좌석 lock 중 일부 좌석에 대한 lock을 대기하여야 할 수 있으므로 wait_time을 고려한 선정
Contributor
|
안녕하세요! 이번 주차도 고생 많으셨습니다! 리뷰 진행하도록 하겠습니다! |
pyuseon
reviewed
Feb 3, 2025
Comment on lines
+43
to
+49
| if(keys instanceof Collection<?>){ | ||
| keysArr = ((Collection<?>)keys).toArray(); | ||
| } else if( keys.getClass().isArray() ){ | ||
| keysArr = (Object[]) keys; | ||
| } else { | ||
| keysArr = new Object[] {keys}; | ||
| } |
Contributor
There was a problem hiding this comment.
이부분은 따로 객체를 배열로 반환하는 메서드로 분리하는 것이 좋을것 같습니다.
private Object[] convertToArray(Object keys) {
if (keys instanceof Collection<?>) {
return ((Collection<?>) keys).toArray();
} else if (keys.getClass().isArray()) {
return (Object[]) keys;
}
return new Object[]{keys};
}
Comment on lines
+60
to
+70
| for (RLock lock : lockList) { | ||
| try{ | ||
| if(!lock.tryLock(distributedLock.waitTime(), distributedLock.leaseTime(),distributedLock.timeUnit())){ | ||
| allAvailable = false; | ||
| break; | ||
| } | ||
| } catch (InterruptedException e){ | ||
| allAvailable = false; | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Lock 획득 부분도 따로 메소드로 빼는 것이 좋을 것 같습니다.
그리고 해당 메서드의 리턴값을 boolean으로 설정하고 이 값에 따라 아래 해제 로직을 적용하면 될 것 같아요.
Comment on lines
+87
to
+92
| if (lock.isHeldByCurrentThread()) { | ||
| try { | ||
| lock.unlock(); | ||
| } catch (IllegalMonitorStateException e) { | ||
| log.warn("Lock was already released: {}", lock.getName()); | ||
| } |
Contributor
There was a problem hiding this comment.
해제 로직도
위의
if (lock.isHeldByCurrentThread()) {
lock.unlock();
}
과 중복 되는데요. 예외처리를 포함해 하나의 해제 매서드를 만들어서 리팩토링 하시면 가독성이 좋은 코드가 될것 같아요.
Comment on lines
+20
to
+21
| private static final Long LOCK_WAIT_TIME = 1L; | ||
| private static final Long LOCK_LEASE_TIME = 2L; |
Contributor
There was a problem hiding this comment.
wait time과 lease time 설정은 서비스에따라서 적절한 값이 달라지게 됩니다.
현재는 wait time을 1초로 설정해 주셨는데요. 빠르게 실패하고 다음에 다시 시도해야 하는 API가 아니라 일정 대기를 고려한다면 적절한 시간으로 보입니다. lease time은 트랜잭션이 오래 걸리지 않는 단순 작업이면 2-3초도 적절합니다.
Comment on lines
+153
to
+159
| for (Ticket ticket : ticketList) { | ||
| ticket.setTicketStatus(TicketStatus.RESERVED); | ||
| Sales sales = Sales.builder().price(9000) | ||
| .user(user).ticket(ticket) | ||
| .createBy("system").build(); | ||
| salesRepository.save(sales); | ||
| } |
Contributor
There was a problem hiding this comment.
이 부분은 검증(validation)이라기보다는 예약 완료 처리에 해당하는 로직으로 보입니다.
따로 분리 하시는 것을 추천 드립니다!
Comment on lines
+63
to
+70
| if (lock.isHeldByCurrentThread()) { | ||
| try { | ||
| lock.unlock(); | ||
| } catch (IllegalMonitorStateException e) { | ||
| log.warn("Lock was already released: {}", lock.getName()); | ||
| } | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
여기도 해제 로직은 하나의 메소드로 관리 할 수 있을것 같습니다! 중복 코드를 줄여보세요!
| return false; | ||
| // 순차적으로 락을 해제하도록 변경 | ||
| for (RLock lock : lockList) { | ||
| if (lock.isHeldByCurrentThread()) { |
Contributor
There was a problem hiding this comment.
다른 스레드에서 락 해제하는 것을 방지하기 위해서 isHeldByCurrentThread를 넣어 주셨네요! 👍
Comment on lines
+103
to
+112
| functionalDistributedLock.executeLock("TICKET:", keys, () -> { | ||
| // 예외처리 [ 존재하지 않는 사용자 ] | ||
| Optional<User> optionalUser = userRepository.findByUsername(username); | ||
| if (optionalUser.isEmpty()) { | ||
| throw new UserNotFoundException(); | ||
| } | ||
|
|
||
| User user = optionalUser.get(); | ||
| validateAndReserve(showingId, user, ticketDtoList); | ||
| }); |
Contributor
There was a problem hiding this comment.
현재 이 부분에 Lock이 걸려있는데요. 락이 필요한 부분(티켓 상태 변경, 결제)만 보호하고, 검증 로직과 유저 조회 로직은 락 없이 처리하면 성능적으로 더 효율적 입니다!
Contributor
좋았던 점
아쉬운 점
리뷰포인트
기타 질문
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
redisson 분산락 도입
지난주에 PR merge를 못 해서 지난주 커밋 내용까지 섞여 있습니다. 죄송합니다.
아래 커밋 이후로 리뷰해주시면 될 것 같습니다!
[ feat ] rds-repo module @Version 제거
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
기타 질문