diff --git a/docs/04-experiment.md b/docs/04-experiment.md index c07fad1..39c964e 100644 --- a/docs/04-experiment.md +++ b/docs/04-experiment.md @@ -94,17 +94,20 @@ ## 8. Warm-up -- Warm-up 실행 여부: -- Warm-up 시간: -- 측정에서 제외한 구간: -- 이유: +- Warm-up 실행 여부: 별도의 명시적 Warm-up 미적용 +- 측정에서 제외한 구간: 없음 +- 영향: + - 첫 번째 부하 테스트 구간에서 HikariCP Pending과 p99가 일시적으로 증가했다. + - 이후 테스트에서는 JVM, DB Connection Pool 등이 Warm-up된 상태였을 가능성이 있다. +- 후속 실험: + - 동일 조건 비교 시 사전 Warm-up을 수행하고 측정 구간에서 제외한다. ## 9. Baseline 결과 ### 동기식 Baseline Mock Provider는 Delivery당 100ms의 지연을 발생시키며, -Provider 호춣을 Notification API의 DB Transaction 내부에서 순차적으로 수행했다. +Provider 호출을 Notification API의 DB Transaction 내부에서 순차적으로 수행했다. | VU | RPS | Avg | p95 | p99 | Error Rate | |---:|---:|---:|---:|---:|---:| @@ -124,27 +127,137 @@ Connection 대기로 인해 응답 시간이 급격히 증가했다. ## 10. 개선 내용 -적용한 개선 사항과 적용 이유를 작성한다. +### Experiment 1. Transaction Boundary 분리 + +Baseline에서는 `NotificationService.send()` 전체를 하나의 DB Transaction으로 처리했다. + +```text +Transaction 시작 + ↓ +Notification / Delivery 저장 + ↓ +Mock Provider 순차 호출 + ↓ +Delivery 상태 변경 + ↓ +Transaction Commit +``` + +Mock Provider는 Delivery당 100ms의 지연을 발생시키며, +테스트 사용자는 Push Device 2개, SMS 1개, Email 1개로 총 4번의 Provider 호출이 발생한다. + +따라서 하나의 요청에서 최소 약 400ms 동안 외부 I/O가 발생했으며, +이 시간 동안 DB Transaction과 Connection도 유지되었다. + +Baseline 부하 테스트에서 HikariCP Connection Pool 최대 크기인 10에 도달했고, +30 VU에서는 약 20개, 50 VU에서는 약 40개의 Connection 대기가 발생했다. + +외부 I/O 대기 시간 동안 DB Connection을 점유하지 않도록 +Transaction Boundary를 다음과 같이 분리했다. + +```text +Transaction 1 +- User / Preference / Device 조회 +- Notification / Delivery 저장 +- Commit +- DB Connection 반환 + + ↓ + +Transaction 없음 +- Mock Provider 순차 호출 + + ↓ + +Transaction 2 +- Delivery 결과 저장 +- Notification 상태 변경 +- Commit +``` + +Provider 호출 방식과 지연 시간은 변경하지 않고, +DB Transaction의 범위만 변경하여 Connection 점유가 성능에 미치는 영향을 비교했다. ## 11. 개선 후 결과 -| 지표 | Baseline | Experiment | 변화 | -|---|---:|---:|---:| -| RPS | | | | -| p95 | | | | -| p99 | | | | -| Error Rate | | | | -| CPU | | | | -| Heap | | | | -| DB Query | | | | +### Experiment 1. Transaction Boundary 분리 결과 + +| VU | 지표 | Baseline | Experiment | 변화 | +|---:|---|---:|---:|---:| +| 30 | RPS | 22.09 | 65.88 | 2.98배 | +| 30 | Avg | 1.33s | 454.66ms | 65.8% 감소 | +| 30 | p95 | 2.13s | 560.98ms | 73.7% 감소 | +| 30 | p99 | 2.57s | 1.47s | 42.8% 감소 | +| 30 | Error Rate | 0% | 0% | 동일 | +| 50 | RPS | 23.55 | 117.74 | 5.00배 | +| 50 | Avg | 2.06s | 421.79ms | 79.5% 감소 | +| 50 | p95 | 3.80s | 456.86ms | 88.0% 감소 | +| 50 | p99 | 3.83s | 515.22ms | 86.5% 감소 | +| 50 | Error Rate | 0% | 0% | 동일 | + +Baseline에서는 동시 요청 증가에 따라 HikariCP Active Connection이 +최댓값인 10에 지속적으로 도달했고 Connection Pending이 증가했다. + +Transaction Boundary 분리 후에는 Provider 호출 중 DB Connection을 반환하면서 +Connection Pool의 지속적인 포화가 사라졌다. + + ## 12. 결과 분석 -- 가설이 맞았는가? -- 어떤 지표가 개선됐는가? -- 어떤 지표는 개선되지 않았는가? -- 새로운 병목은 어디에서 발생했는가? -- 측정 결과에 영향을 준 외부 요인은 무엇인가? +### 12.1 DB Connection Pool 병목 확인 + +Baseline에서는 VU를 30에서 50으로 증가시켜도 RPS가 +22.09에서 23.55로 약 6.6% 증가하는 데 그쳤다. + +반면 p95는 2.13초에서 3.80초로 증가하여, +동시 요청 증가가 처리량 증가보다 대기 시간 증가로 이어졌다. + +HikariCP의 최대 Connection 수는 10이었으며, +Provider 호출을 포함한 Transaction이 약 400ms 이상 Connection을 점유했다. + +따라서 이론적인 처리량 한계는 대략 다음과 같이 예상할 수 있다. + +```text +10 Connections / 약 0.42초 +≈ 23.8 RPS +``` + +실제 50 VU Baseline의 처리량은 23.55 RPS로, +Connection Pool에 의해 처리량이 제한되었다는 분석과 유사한 결과를 보였다. + +### 12.2 Transaction Boundary 분리 효과 + +외부 Provider 호출을 DB Transaction 밖으로 분리한 후 +50 VU에서 RPS는 23.55에서 117.74로 약 5배 증가했다. + +p95 역시 3.80초에서 456.86ms로 약 88% 감소했다. + +Provider 호출 중에는 DB Connection을 점유하지 않으므로, +동시 요청이 증가하더라도 Connection Pool 대기가 지속적으로 발생하지 않았다. + +50 VU에서 평균 응답 시간이 약 422ms였으므로 다음과 같이 예상할 수 있다. + +```text +50 VU / 약 0.422초 +≈ 118 RPS +``` +실제 처리량인 117.74 RPS와 유사핟. + +이를 통해 Transcation Boundary 분리 이후에는 DB Connection Pool보다 +동기식 Provider 호출 시간이 처리량과 응답 시간에 더 직접적인 영향을 주는 것으로 판단했다. + +### 12.3 남아 있는 병목 + +DB Connection Pool 병목은 완화되었지만, +Notification API는 여전히 모든 Provider 호출이 끝날 때까지 HTTP 응답을 반환하지 않는다. + +현재 한 요청은 총 4번의 Mock Provider를 순차적으로 호출하므로 +최소 약 400ms의 응답 시간이 발생한다. + +따라서 다음 실험에서는 RabbitMQ를 이용하여 +Provider 호출을 HTTP 요청 처리 경로에서 분리하고, +API가 알림 요청을 Queue에 등록한 뒤 즉시 응답하도록 비동기 구조로 변경한다. ## 13. Platform Thread와 Virtual Thread 비교 diff --git a/docs/images/transaction-boundary.png b/docs/images/transaction-boundary.png new file mode 100644 index 0000000..5811a15 Binary files /dev/null and b/docs/images/transaction-boundary.png differ diff --git a/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryCommand.java b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryCommand.java new file mode 100644 index 0000000..2e45415 --- /dev/null +++ b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryCommand.java @@ -0,0 +1,10 @@ +package com.backendsystemdesignlab.notification.notification.dto; + +import com.backendsystemdesignlab.notification.user.domain.NotificationChannel; + +public record DeliveryCommand( + Long deliveryId, + NotificationChannel channel, + String destination +) { +} diff --git a/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryResult.java b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryResult.java new file mode 100644 index 0000000..c76256d --- /dev/null +++ b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/DeliveryResult.java @@ -0,0 +1,7 @@ +package com.backendsystemdesignlab.notification.notification.dto; + +public record DeliveryResult( + Long deliveryId, + boolean success +) { +} diff --git a/src/main/java/com/backendsystemdesignlab/notification/notification/dto/PreparedNotification.java b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/PreparedNotification.java new file mode 100644 index 0000000..4d98a1e --- /dev/null +++ b/src/main/java/com/backendsystemdesignlab/notification/notification/dto/PreparedNotification.java @@ -0,0 +1,17 @@ +package com.backendsystemdesignlab.notification.notification.dto; + +import com.backendsystemdesignlab.notification.notification.domain.NotificationStatus; + +import java.util.List; + +public record PreparedNotification( + Long notificationId, + NotificationStatus status, + List deliveries, + boolean alreadyProcessed +) { + + public long deliveryCount() { + return deliveries.size(); + } +} diff --git a/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationService.java b/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationService.java index e73acea..52ed969 100644 --- a/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationService.java +++ b/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationService.java @@ -1,177 +1,85 @@ package com.backendsystemdesignlab.notification.notification.service; -import com.backendsystemdesignlab.notification.notification.domain.DeliveryStatus; -import com.backendsystemdesignlab.notification.notification.domain.Notification; -import com.backendsystemdesignlab.notification.notification.domain.NotificationDelivery; -import com.backendsystemdesignlab.notification.notification.dto.SendNotificationRequest; -import com.backendsystemdesignlab.notification.notification.dto.SendNotificationResponse; +import com.backendsystemdesignlab.notification.notification.domain.NotificationStatus; +import com.backendsystemdesignlab.notification.notification.dto.*; import com.backendsystemdesignlab.notification.notification.provider.EmailProvider; import com.backendsystemdesignlab.notification.notification.provider.ProviderResult; import com.backendsystemdesignlab.notification.notification.provider.PushProvider; import com.backendsystemdesignlab.notification.notification.provider.SmsProvider; -import com.backendsystemdesignlab.notification.notification.repository.NotificationDeliveryRepository; -import com.backendsystemdesignlab.notification.notification.repository.NotificationRepository; -import com.backendsystemdesignlab.notification.user.domain.NotificationChannel; -import com.backendsystemdesignlab.notification.user.domain.NotificationPreference; -import com.backendsystemdesignlab.notification.user.domain.User; -import com.backendsystemdesignlab.notification.user.domain.UserDevice; -import com.backendsystemdesignlab.notification.user.repository.NotificationPreferenceRepository; -import com.backendsystemdesignlab.notification.user.repository.UserDeviceRepository; -import com.backendsystemdesignlab.notification.user.repository.UserRepository; + import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import java.util.ArrayList; import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; @Service @RequiredArgsConstructor public class NotificationService { - private final UserRepository userRepository; - private final UserDeviceRepository userDeviceRepository; - private final NotificationPreferenceRepository preferenceRepository; - private final NotificationRepository notificationRepository; - private final NotificationDeliveryRepository deliveryRepository; + private final NotificationTransactionService transactionService; + private final PushProvider pushProvider; private final SmsProvider smsProvider; private final EmailProvider emailProvider; - @Transactional public SendNotificationResponse send(SendNotificationRequest request) { - // 동일 eventId가 이미 처리된 경우 기존 결과 반환 - var existing = notificationRepository.findByEventId(request.eventId()); - - if (existing.isPresent()) { - Notification notification = existing.get(); + // DB 작업 + PreparedNotification prepared = transactionService.prepare(request); + // 이미 처리했던 eventId + if (prepared.alreadyProcessed()) { + long deliveryCount = transactionService.countDeliveries(prepared.notificationId()); return new SendNotificationResponse( - notification.getId(), - notification.getStatus(), - deliveryRepository.countByNotificationId(notification.getId()) + prepared.notificationId(), + prepared.status(), + deliveryCount ); } - User user = userRepository.findById(request.userId()) - .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다.")); - - Set enabledChannels = preferenceRepository.findAllByUserIdAndEnabledTrue(user.getId()) - .stream() - .map(NotificationPreference::getChannel) - .collect(Collectors.toSet()); - - Notification notification = notificationRepository.save(new Notification(request.eventId(), user)); - - List deliveries = new ArrayList<>(); - - for (NotificationChannel channel : request.channels()) { - - if (!enabledChannels.contains(channel)) { - continue; - } - - switch (channel) { - case PUSH -> createPushDeliveries( - user, - notification, - deliveries - ); - - case SMS -> createSmsDelivery( - user, - notification, - deliveries - ); - - case EMAIL -> createEmailDelivery( - user, - notification, - deliveries - ); - } - } - - deliveryRepository.saveAll(deliveries); - - notification.startProcessing(); + // Provider 호출 (DB 트랜잭션을 사용하지 않음) + List results = sendDeliveries(prepared.deliveries()); - sendDeliveries(deliveries); + // DB 작업 + transactionService.complete(prepared.notificationId(), results); - boolean allSucceeded = deliveries.stream() - .allMatch(delivery -> delivery.getStatus() == DeliveryStatus.SENT); - - if (allSucceeded) { - notification.complete(); - } else { - notification.fail(); - } + boolean allSucceeded = results.stream().allMatch(DeliveryResult::success); return new SendNotificationResponse( - notification.getId(), - notification.getStatus(), - deliveries.size() + prepared.notificationId(), + allSucceeded + ? NotificationStatus.COMPLETED + : NotificationStatus.FAILED, + prepared.deliveryCount() ); + } - private void sendDeliveries(List deliveries) { + private List sendDeliveries(List deliveries) { - for (NotificationDelivery delivery : deliveries) { - delivery.recordAttempt(); + List results = new ArrayList<>(); - ProviderResult result = switch (delivery.getChannel()) { - case PUSH -> pushProvider.send(delivery.getDestination()); - case SMS -> smsProvider.send(delivery.getDestination()); - case EMAIL -> emailProvider.send(delivery.getDestination()); - }; + for (DeliveryCommand delivery : deliveries) { + boolean success; - if (result.success()) { - delivery.markSent(); - } else { - delivery.markFailed(); + try { + ProviderResult result = + switch (delivery.channel()) { + case PUSH -> pushProvider.send(delivery.destination()); + case SMS -> smsProvider.send(delivery.destination()); + case EMAIL -> emailProvider.send(delivery.destination()); + }; + success = result.success(); + } catch (RuntimeException e) { + success = false; } - } - } - - private void createPushDeliveries(User user, Notification notification, List deliveries) { - List devices = userDeviceRepository.findAllByUserIdAndActiveTrue(user.getId()); - for (UserDevice device : devices) { - deliveries.add( - new NotificationDelivery( - notification, - NotificationChannel.PUSH, - device.getDeviceToken() - ) - ); + results.add(new DeliveryResult(delivery.deliveryId(), success)); } - } - - private void createSmsDelivery(User user, Notification notification, List deliveries) { - if (user.getPhoneNumber() == null || user.getPhoneNumber().isBlank()) return; - deliveries.add( - new NotificationDelivery( - notification, - NotificationChannel.SMS, - user.getPhoneNumber() - ) - ); + return results; } - private void createEmailDelivery(User user, Notification notification, List deliveries) { - if (user.getEmail() == null || user.getEmail().isBlank()) return; - - deliveries.add( - new NotificationDelivery( - notification, - NotificationChannel.EMAIL, - user.getEmail() - ) - ); - } } diff --git a/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationTransactionService.java b/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationTransactionService.java new file mode 100644 index 0000000..73cb3ce --- /dev/null +++ b/src/main/java/com/backendsystemdesignlab/notification/notification/service/NotificationTransactionService.java @@ -0,0 +1,191 @@ +package com.backendsystemdesignlab.notification.notification.service; + +import com.backendsystemdesignlab.notification.notification.domain.Notification; +import com.backendsystemdesignlab.notification.notification.domain.NotificationDelivery; +import com.backendsystemdesignlab.notification.notification.dto.DeliveryCommand; +import com.backendsystemdesignlab.notification.notification.dto.DeliveryResult; +import com.backendsystemdesignlab.notification.notification.dto.PreparedNotification; +import com.backendsystemdesignlab.notification.notification.dto.SendNotificationRequest; +import com.backendsystemdesignlab.notification.notification.repository.NotificationDeliveryRepository; +import com.backendsystemdesignlab.notification.notification.repository.NotificationRepository; +import com.backendsystemdesignlab.notification.user.domain.NotificationChannel; +import com.backendsystemdesignlab.notification.user.domain.NotificationPreference; +import com.backendsystemdesignlab.notification.user.domain.User; +import com.backendsystemdesignlab.notification.user.domain.UserDevice; +import com.backendsystemdesignlab.notification.user.repository.NotificationPreferenceRepository; +import com.backendsystemdesignlab.notification.user.repository.UserDeviceRepository; +import com.backendsystemdesignlab.notification.user.repository.UserRepository; +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +@Service +@RequiredArgsConstructor +public class NotificationTransactionService { + + private final UserRepository userRepository; + private final UserDeviceRepository userDeviceRepository; + private final NotificationPreferenceRepository preferenceRepository; + private final NotificationRepository notificationRepository; + private final NotificationDeliveryRepository deliveryRepository; + + @Transactional + public PreparedNotification prepare(SendNotificationRequest request) { + + var existing = notificationRepository.findByEventId(request.eventId()); + + if (existing.isPresent()) { + Notification notification = existing.get(); + + return new PreparedNotification( + notification.getId(), + notification.getStatus(), + List.of(), + true + ); + } + + User user = userRepository.findById(request.userId()) + .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다.")); + + Set enabledChannels = preferenceRepository.findAllByUserIdAndEnabledTrue(user.getId()) + .stream() + .map(NotificationPreference::getChannel) + .collect(Collectors.toSet()); + + Notification notification = notificationRepository.save(new Notification(request.eventId(), user)); + + List deliveries = new ArrayList<>(); + + for (NotificationChannel channel : request.channels()) { + + if (!enabledChannels.contains(channel)) { + continue; + } + + switch (channel) { + case PUSH -> createPushDeliveries( + user, + notification, + deliveries + ); + + case SMS -> createSmsDelivery( + user, + notification, + deliveries + ); + + case EMAIL -> createEmailDelivery( + user, + notification, + deliveries + ); + } + } + + deliveryRepository.saveAll(deliveries); + + deliveryRepository.flush(); // DB의 ID를 얻기 위함 (delivery.getId()) + + notification.startProcessing(); + + List commands = deliveries.stream() + .map(delivery -> new DeliveryCommand(delivery.getId(), delivery.getChannel(), delivery.getDestination())).toList(); + + return new PreparedNotification( + notification.getId(), + notification.getStatus(), + commands, + false + ); + } + + @Transactional + public void complete(Long notificationId, List results) { + + // 기존 notification 객체를 쓰지 않는 이유는 첫 번째 Transaction이 끝났기 때문에 두 번째 Transaction에서는 새 영속성 컨텍스트에서 다시 조회 + Notification notification = notificationRepository.findById(notificationId) + .orElseThrow(() -> new IllegalArgumentException("알림을 찾을 수 없습니다.")); + + List deliveryIds = results.stream().map(DeliveryResult::deliveryId).toList(); + + Map deliveryMap = deliveryRepository.findAllById(deliveryIds) + .stream() + .collect(Collectors.toMap(NotificationDelivery::getId, Function.identity())); + + for (DeliveryResult result : results) { + NotificationDelivery delivery = deliveryMap.get(result.deliveryId()); + + if (delivery == null) { + throw new IllegalStateException("전송 정보를 찾을 수 없습니다. id=" + result.deliveryId()); + } + + delivery.recordAttempt(); + + if (result.success()) { + delivery.markSent(); + } else { + delivery.markFailed(); + } + } + + boolean allSucceeded = results.stream().allMatch(DeliveryResult::success); + + if (allSucceeded) { + notification.complete(); + } else { + notification.fail(); + } + } + + @Transactional(readOnly = true) + public long countDeliveries(Long notificationId) { + return deliveryRepository.countByNotificationId(notificationId); + } + + private void createPushDeliveries(User user, Notification notification, List deliveries) { + List devices = userDeviceRepository.findAllByUserIdAndActiveTrue(user.getId()); + + for (UserDevice device : devices) { + deliveries.add( + new NotificationDelivery( + notification, + NotificationChannel.PUSH, + device.getDeviceToken() + ) + ); + } + } + + private void createSmsDelivery(User user, Notification notification, List deliveries) { + if (user.getPhoneNumber() == null || user.getPhoneNumber().isBlank()) return; + + deliveries.add( + new NotificationDelivery( + notification, + NotificationChannel.SMS, + user.getPhoneNumber() + ) + ); + } + + private void createEmailDelivery(User user, Notification notification, List deliveries) { + if (user.getEmail() == null || user.getEmail().isBlank()) return; + + deliveries.add( + new NotificationDelivery( + notification, + NotificationChannel.EMAIL, + user.getEmail() + ) + ); + } +}