feat: send periodic heartbeat to keep notification SSE connections alive - #62
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSpring 스케줄링을 활성화했습니다. 전체 ChangesSSE heartbeat 전송
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SpringScheduler
participant NotificationSseHeartbeatScheduler
participant NotificationEmitterRegistry
participant SseEmitter
SpringScheduler->>NotificationSseHeartbeatScheduler: 30초 주기 sendHeartbeat() 실행
NotificationSseHeartbeatScheduler->>NotificationEmitterRegistry: findAll() 호출
NotificationEmitterRegistry-->>NotificationSseHeartbeatScheduler: 전체 SseEmitter 목록 반환
loop 각 SSE 연결
NotificationSseHeartbeatScheduler->>SseEmitter: heartbeat comment 전송
alt IOException 또는 IllegalStateException 발생
NotificationSseHeartbeatScheduler->>SseEmitter: 오류와 함께 연결 종료
end
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
momogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java (2)
24-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win[권장] heartbeat 성공·실패·주기 회귀 테스트를 추가하세요.
PR objective에서 테스트가 미완료로 표시되어 있습니다. 최소한 다음 사례를 검증하세요.
- 여러 사용자와 여러 emitter가 모두
heartbeatcomment를 받는지 확인합니다.- 한 emitter의
IOException또는IllegalStateException이 다음 emitter 전송을 중단하지 않는지 확인합니다.- lifecycle callback 이후
findAll()에서 emitter가 제외되는지 확인합니다.- 실제 60초 이상 대기하지 말고, 짧은 주기 또는 제어 가능한 scheduler를 사용합니다.
직접 호출하는 unit test는 빠르고 결정적입니다. Application context 기반 test는
@EnableScheduling과 component scanning을 검증하지만 실행 시간이 길고 더 취약합니다.As per path instructions:
**/main/**/*.java파일은 Java 21 기준으로 리뷰하며, 객체지향·실무 활용성·불필요한 로직·클린코드·서비스/도메인 설계·공통 예외 처리를 확인합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java` around lines 24 - 33, The NotificationSseHeartbeatScheduler tests are missing coverage for heartbeat delivery and scheduling behavior. Add fast, deterministic unit tests around sendHeartbeat() verifying all emitters receive the heartbeat comment, IOException and IllegalStateException from one emitter do not prevent subsequent sends, lifecycle callbacks remove emitters from emitterRegistry.findAll(), and the interval is tested through a short or controllable scheduler instead of waiting 60 seconds.Source: Path instructions
20-24: 🩺 Stability & Availability | 🔵 Trivial[운영 설정] SSE 하트비트 주기와 가장 짧은 idle timeout을 대조하세요.
SseEmitter는 30분으로 설정되어 있으므로 애플리케이션 내 timeout 문제는 없습니다. 하지만 배포 경로는 Nginx 재적재와ALB_DNS_NAME을 사용하므로, Nginx, ALB 또는 JVM 프로세스 자체의read_timeout/idle timeout이 30초 이하일 수 있습니다. SSE 연결이 하트비트보다 먼저 끊어지지 않도록 최소 1.5x 여유로 맞춰 배포 설정을 확인하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java` around lines 20 - 24, Verify the deployment path settings used by NotificationSseHeartbeatScheduler, including Nginx, ALB, and JVM process read/idle timeouts, and configure the shortest timeout to be at least 1.5 times HEARTBEAT_INTERVAL so SSE connections are not closed before a heartbeat.Sources: Path instructions, MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@momogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java`:
- Around line 26-32: Update the heartbeat loop in
NotificationSseHeartbeatScheduler to catch IllegalStateException alongside
IOException so a terminated emitter cannot stop broadcasting to remaining
emitters. Remove emitter.completeWithError(e) from the IOException handling and
retain warning logging while allowing the registry’s existing onError cleanup to
handle removal.
- Around line 24-28: Update sendHeartbeat() to submit each emitter’s send
operation to a bounded executor instead of performing sends serially in the
scheduled thread. Ensure each emitter has at most one in-flight heartbeat task,
while allowing different emitters to be processed independently and preserving
the existing heartbeat event and failure handling.
---
Nitpick comments:
In
`@momogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java`:
- Around line 24-33: The NotificationSseHeartbeatScheduler tests are missing
coverage for heartbeat delivery and scheduling behavior. Add fast, deterministic
unit tests around sendHeartbeat() verifying all emitters receive the heartbeat
comment, IOException and IllegalStateException from one emitter do not prevent
subsequent sends, lifecycle callbacks remove emitters from
emitterRegistry.findAll(), and the interval is tested through a short or
controllable scheduler instead of waiting 60 seconds.
- Around line 20-24: Verify the deployment path settings used by
NotificationSseHeartbeatScheduler, including Nginx, ALB, and JVM process
read/idle timeouts, and configure the shortest timeout to be at least 1.5 times
HEARTBEAT_INTERVAL so SSE connections are not closed before a heartbeat.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59785e50-ded6-44b3-a421-6bef04b1a5ea
📒 Files selected for processing (3)
momogo-api/src/main/java/com/momogo/api/MomogoApiApplication.javamomogo-api/src/main/java/com/momogo/api/notification/registry/NotificationEmitterRegistry.javamomogo-api/src/main/java/com/momogo/api/notification/scheduler/NotificationSseHeartbeatScheduler.java
Junkov0
approved these changes
Aug 7, 2026
Junkov0
left a comment
Junkov0
left a comment
Collaborator
There was a problem hiding this comment.
따로 수정사항이 없는 것 같습니다.
작성 하느라 수고많으셨습니다!
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
3 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.
작업 내용
변경 사항
체크리스트
참고 사항
관련 이슈
Summary by CodeRabbit