-
Notifications
You must be signed in to change notification settings - Fork 0
[MOISAM-247] 경로 조회 재시도 로직 추가 및 Virtual Thread로 전환한다 #174
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
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough이벤트 캐시 존재 검증 메서드를 제거하고, RouteAssembler의 라우트 조회를 가상 스레드 기반 공유 Executor로 전환해 재시도 로직(fetchWithRetry)과 유효성 검사(isValid)를 도입했습니다. 새로운 ThreadConfig에서 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-10-11T14:28:08.219ZApplied to files:
📚 Learning: 2025-09-28T08:19:04.426ZApplied to files:
📚 Learning: 2025-09-25T13:46:49.071ZApplied to files:
📚 Learning: 2025-09-28T08:19:04.426ZApplied to files:
📚 Learning: 2025-12-25T11:50:19.070ZApplied to files:
🧬 Code graph analysis (1)src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java (1)
56-73: 마지막 시도 실패 시 로그 추가 고려현재 구현에서는 재시도 전에만 로그가 남고, 모든 시도가 실패하여
null을 반환할 때는 로그가 없습니다. 디버깅을 위해 최종 실패 시에도 로그를 남기는 것을 권장합니다.♻️ Proposed fix
private RouteResponse fetchWithRetry(StartPoint startPoint, Subway subway) { for (int attempt = 0; attempt < MAX_ATTEMPTS; attempt++) { RouteResponse route = routeFetcher.fetch(startPoint, subway); if (isValid(route)) return route; if (attempt < MAX_ATTEMPTS - 1) { log.warn("[RouteAssembler] Route fetch failed for {}. Retrying... (Attempt {}/{})", startPoint.getName(), attempt + 1, MAX_ATTEMPTS); try { Thread.sleep(RETRY_DELAY_MS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new EventException(EventErrorType.ROUTE_FETCH_FAILED); } } } + log.error("[RouteAssembler] All {} attempts failed for startPoint: {}, subway: {}", + MAX_ATTEMPTS, startPoint.getName(), subway.getName()); return null; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/meetup/server/event/implement/EventValidator.javasrc/main/java/com/meetup/server/event/implement/route/RouteAssembler.javasrc/main/java/com/meetup/server/global/config/ThreadConfig.java
💤 Files with no reviewable changes (1)
- src/main/java/com/meetup/server/event/implement/EventValidator.java
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java:25-34
Timestamp: 2025-09-28T08:27:38.024Z
Learning: MOISAM 프로젝트의 외부 API(Odsay Transit, Kakao Mobility)는 동시 호출 제한이 있어서 3개 이상의 스레드로 병렬 요청 시 일부 경로 조회가 실패함. 스테이징 테스트 결과 2개 스레드가 안정적으로 동작하는 최적값으로 확인됨.
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java:30-35
Timestamp: 2025-09-25T13:46:49.071Z
Learning: MOISAM 프로젝트에서 RouteProcessor + RouteAssembler의 중첩 Virtual Thread 사용은 EventValidator의 최대 출발지 8개 제한과 중간지점 알고리즘 특성으로 최대 24-32개 동시 API 호출로 제한되며, RateLimiter 인프라와 캐싱 전략으로 적절히 관리됨.
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteProcessor.java:32-38
Timestamp: 2025-09-25T13:43:29.955Z
Learning: RouteProcessor.buildRouteGroups() 메서드에서 중간지점역 리스트 수 × 출발지 수만큼 동시 API 호출이 발생하여 외부 API 레이트 리밋에 도달할 수 있는 중첩 병렬화 문제가 있음.
📚 Learning: 2025-10-11T14:28:08.219Z
Learnt from: syjdjr
Repo: Team-MOISAM/moisam-server PR: 162
File: src/main/java/com/meetup/server/event/application/EventService.java:68-87
Timestamp: 2025-10-11T14:28:08.219Z
Learning: In the moisam-server codebase, the EventService.getMeetingPointRoutes method uses a ReentrantLock via EventLockManager that should block and wait (using lock.lock()) rather than using tryLock with timeout. The 409 error handling for concurrent access applies to StartPointService but not to this particular lock in EventService.
Applied to files:
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java
📚 Learning: 2025-09-28T08:19:04.426Z
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteProcessor.java:32-55
Timestamp: 2025-09-28T08:19:04.426Z
Learning: RouteAssembler.assemble() 메서드는 예외가 발생해도 CompletableFuture가 exceptionally 완료되지 않고, 내부에서 예외를 catch하여 null로 변환하여 정상 완료됨. 따라서 CompletableFuture.allOf()를 사용해도 개별 assemble 실패가 전체 처리를 중단시키지 않음.
Applied to files:
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java
📚 Learning: 2025-09-25T13:46:49.071Z
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java:30-35
Timestamp: 2025-09-25T13:46:49.071Z
Learning: MOISAM 프로젝트에서 RouteProcessor + RouteAssembler의 중첩 Virtual Thread 사용은 EventValidator의 최대 출발지 8개 제한과 중간지점 알고리즘 특성으로 최대 24-32개 동시 API 호출로 제한되며, RateLimiter 인프라와 캐싱 전략으로 적절히 관리됨.
Applied to files:
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java
📚 Learning: 2025-09-28T08:19:04.426Z
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 160
File: src/main/java/com/meetup/server/event/implement/route/RouteProcessor.java:32-55
Timestamp: 2025-09-28T08:19:04.426Z
Learning: RouteFetcher.fetch() 메서드는 개별 API 호출(fetchTransitRoute, fetchDrivingRoute)에서 예외를 catch하여 null로 변환하므로, 전체 메서드가 정상적으로 RouteResponse를 반환함. 따라서 상위의 CompletableFuture 체인에서 exceptionally 완료되지 않고 정상 완료됨.
Applied to files:
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java
📚 Learning: 2025-12-25T11:50:19.070Z
Learnt from: anxi01
Repo: Team-MOISAM/moisam-server PR: 170
File: src/main/java/com/meetup/server/admin/application/AdminService.java:70-76
Timestamp: 2025-12-25T11:50:19.070Z
Learning: In the MOISAM project, the server and database operate in the Asia/Seoul timezone, and the service is domestic. Therefore, using LocalDate.now() or LocalDateTime.now() without an explicit ZoneId is acceptable for this codebase. Apply this guideline to Java files under src/main/java, but remain vigilant: for time-sensitive data or if future maintenance requires cross-timezone handling, prefer using ZoneId or ZonedDateTime with an explicitZone.
Applied to files:
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.javasrc/main/java/com/meetup/server/global/config/ThreadConfig.java
🧬 Code graph analysis (1)
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java (1)
src/main/java/com/meetup/server/event/exception/EventException.java (1)
EventException(6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/java/com/meetup/server/event/implement/route/RouteAssembler.java (3)
26-31: LGTM!재시도 상수와 executor 주입이 적절하게 설정되어 있습니다. Virtual Thread는
Thread.sleep()시 unmount되어 다른 Virtual Thread가 실행될 수 있으므로, 재시도 로직에 적합한 선택입니다.
75-79: LGTM!
isTransit플래그와 해당 route 존재 여부를 함께 검증하는 로직이 적절합니다. Learning에 따르면RouteFetcher.fetch()가 예외를null로 변환하므로 이 유효성 검사가 필수적입니다.
47-53: null 반환은 호출자에서 적절히 처리됨
RouteProcessor.buildRouteGroups()에서assemble()호출 결과에 대해.filter(Objects::nonNull)으로 명시적으로 null을 필터링하고 있습니다(Line 47). 따라서routes.isEmpty()케이스의 null 반환은 호출자에서 적절히 처리되고 있습니다.src/main/java/com/meetup/server/global/config/ThreadConfig.java (1)
13-17: ExecutorService 라이프사이클은 Spring 6.1+에서 자동으로 관리됩니다현재 프로젝트는 Spring Boot 3.4.4를 사용하고 있으며, 이는 Spring Framework 6.1 이상에 해당합니다. Spring 6.1+에서는
AutoCloseable을 구현하는 빈들을 자동으로 종료하므로,Executors.newThreadPerTaskExecutor()로 생성된ExecutorService는 별도의 shutdown 처리 없이 자동으로 관리됩니다. Java 21에서 제공되는 Virtual Thread 기반 ExecutorService도 동일하게 작동합니다. 추가 라이프사이클 관리는 필요하지 않습니다.
🚀 Why - 해결하려는 문제가 무엇인가요?
출발지 수가 6개 이상 + 중간지점 수 2개 이상일 경우, 1개의 중간지점의 경로 조회가 대부분 실패합니다.
이 경우를 대처하기 위해 작업을 진행했습니다.
✅ What - 무엇이 변경됐나요?
get + try-catch에서 join으로 변경🛠️ How - 어떻게 해결했나요?
🖼️ Attachment
💬 기타 코멘트
Summary by CodeRabbit
새로운 기능
성능 개선
개선 사항
수정
✏️ Tip: You can customize this high-level summary in your review settings.