-
Notifications
You must be signed in to change notification settings - Fork 0
Group wise 30: N+1 문제 해결을 위한 배치사이즈 설정, 알림리스너 개선 #35
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
Conversation
|
|
|
||
| // 관심 회원들에게 알림 | ||
| Map<Boolean, Long> wishlistResults = groupPurchase.getWishlistIds().parallelStream() | ||
| .map(wishlistId -> notificationService.notify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map 로직 안에서 exception이 발생한 경우는 어떻게 동작하나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 질문 주셨는데 답변이 너무 늦었습니다. ㅠ 찾아보면서 아래와 같은 답을 얻었습니다.
map 로직 안에서 notificationService.notify() 를 호출하고 있는데, 메서드 내부에서 로직이 전부 try-catch 로 감싸져있습니다. 그리고 exception 은 catch에서 받아 false를 반환하게 됩니다. (알림 발송 실패를 뜻함)
하지만 만약 내부에서 catch하지 않고 exception이 바깥으로 던져지게 된다면?
- 이미 발송된 알림은 롤백되지 않습니다
- 그리고 실패 이후의 모든 알림 동작도 누락되게 됩니다
- 그 이유는 GroupPurchaseEventListener 에는 @transactional 이 선언되어있지 않아 트랜젝션 경게에 포함되지 않고, notify() 메서드부터 개별적으로 트랜젝션이 생기기 때문입니다.
그리고 앞으로 더 개선해야할 부분에 대해서도 생각해보았습니다.
- try-catch 가 notify 메서드 안쪽에 있는 것보다 map 호출부에 있는 것이 더 낫다는 점 알게 됐습니다.
- 메서드 바깥 호출부에서 실패한 사용자id 를 확인할 수 있고, 알림 발송이 실패한 사용자 id에 대해 로깅하거나, 알림을 재시도할 수 있기 때문입니다.
| participantResults.getOrDefault(true, 0L), participantResults.getOrDefault(false, 0L), | ||
| wishlistResults.getOrDefault(true, 0L), wishlistResults.getOrDefault(false, 0L)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
참여자들과 관심 회원들에게 알리는 것이 순차적으로 동작할 필요가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지난번 멘토링 시간에 잠깐 언급이 되었던 것처럼, CompletableFuture를 사용해서 모두 비동기로 처리되도록 변경하겠습니다!
| notificationService.notify( | ||
| new Notification( | ||
|
|
||
| Map<Boolean, Long> results = groupPurchase.getWishlistIds().parallelStream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 parallelStream을 활용하면 어떤 스레드에서 돌게 되는 건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
찾아보니,
parallelStream()은 어디에서 사용하든, 호출한 스레드 + ForkJoinPool 을 이용하는 것이었습니다.
parallelStream 과 CompletableFuture 의 스레드 풀 사용
- 현재 호출되는 곳에서 @async("notificationTaskExecutor") 를 사용하고 있으므로
notificationTaskExecutor에서 parallelStream()을 호출한 스레드 1개 + 나머지 작업은 ForkJoinPool 의 스레드에서 처리합니다. - 반면, CompletableFuture 를 사용하게 된다면, executor 를 지정할 수 있습니다.
현재의 문제점과 개선사항
결국 현재 AsyncConfig 에서 정의한 TaskExecutor 가 제대로 사용되지 않는 것이었네요..
CompletableFuture 를 사용하고, 사용할 executor를 잘 지정해주는 것으로 변경해야겠습니다!
(지정하지 않으면 기본적으로 ForkJoinPool 을 사용함)
ForkJoinPool vs TaskPoolExecutor
- ForkJoinPool : 분할 정복 방식 처리, WorkStealing 기능 있음, CPU 집약적인 작업에 효율적
- TaskPoolExecutor : 큐 방식 처리, WorkStealing 기능 없음, I/O 바운디드 된 작업에 효율적



작업내용: