fix(push-noti): 공지사항 푸시알림 전송 기능 구현 (2,3) #72
Conversation
- ExecutorService 포화 방지를 위한 배치 처리 및 CallerRunsPolicy 적용
- 실패한 알림도 전송 완료로 표시하여 재시도 방지
There was a problem hiding this comment.
Pull Request Overview
This PR implements resilience improvements for the push notification system to prevent thread pool exhaustion and handle notification failures more gracefully.
- Adds a
CallerRunsPolicyrejection handler to the async executor configuration - Marks failed notifications as sent to prevent infinite retry loops
- Implements batch processing with delays for sending notifications to all users
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| AsyncConfig.java | Adds a rejection execution handler to prevent task rejection when thread pool is saturated |
| PushNotificationService.java | Updates failure handling to mark failed notifications as sent, preventing retry loops |
| NotificationTemplateService.java | Introduces batch processing with inter-batch delays to avoid overwhelming the thread pool |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| sentCount++; | ||
| } else { | ||
| final int BATCH_SIZE = 20; // 배치 크기 설정 (스레드 풀 용량 35개 고려) |
There was a problem hiding this comment.
The comment mentions thread pool capacity of 35, but the AsyncConfig shows maxPoolSize=10 and queueCapacity=25 (total capacity of 35). However, this is misleading since the queue only holds pending tasks, not active threads. The actual concurrent execution capacity is 10 threads. Consider clarifying this as '스레드 풀 총 용량 35개 (최대 스레드 10개 + 큐 25개) 고려' to accurately reflect the distinction between thread capacity and queue capacity.
| final int BATCH_SIZE = 20; // 배치 크기 설정 (스레드 풀 용량 35개 고려) | |
| final int BATCH_SIZE = 20; // 배치 크기 설정 (스레드 풀 총 용량 35개 [최대 스레드 10개 + 큐 25개] 고려) |
| log.debug("배치 처리 중: {}/{} (배치 크기: {})", | ||
| endIndex, activeUsers.size(), batch.size()); | ||
|
|
||
| /// 푸시 알림 전송 |
There was a problem hiding this comment.
[nitpick] This comment uses triple slashes (///) which is inconsistent with the double slash (//) comment style used elsewhere in the same method (lines 189). The codebase consistently uses triple slashes for section markers and field documentation. For inline section comments within methods, consider using double slashes for consistency, or establish a clear convention.
| skippedCount++; | ||
| } | ||
|
|
||
| /// 배치 간 대기 (스레드 풀 여유 확보) |
There was a problem hiding this comment.
[nitpick] This comment uses triple slashes (///) which is inconsistent with the single-line comment style used in line 167 of the same method. The codebase consistently uses triple slashes for section markers and field documentation. For inline section comments within methods, consider using double slashes for consistency, or establish a clear convention.
| /// 배치 간 대기 (스레드 풀 여유 확보) | ||
| if (endIndex < activeUsers.size()) { | ||
| try { | ||
| TimeUnit.MILLISECONDS.sleep(100); |
There was a problem hiding this comment.
The hardcoded 100ms sleep duration should be extracted as a named constant (e.g., BATCH_DELAY_MILLIS) to improve maintainability and make the purpose clearer. This would also make it easier to tune the delay if needed.
No description provided.