-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] 환승할 때마다 walk step 추가, 공간 필터링을 이용해 OOM 문제 해결 #159
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
Walkthrough대중교통 경로 탐색에 공간 필터링(바운딩 박스) 로딩을 도입하고, 환승 구간에 WALK 스텝을 자동 삽입했습니다. 버스 정보 조회는 외부 API 대신 더미 데이터를 즉시 반환하도록 변경했습니다. Node/Edge 리포지토리에 바운딩 박스 조회 메서드를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant TS as TransportationService
participant NR as NodeRepository
participant ER as EdgeRepository
participant Graph
User->>TS: findMultipleTransportationRoutes(start, end)
TS->>TS: calculateBoundingBox(startNode, endNode)
TS->>NR: findNodesInBoundingBox(minLat,maxLat,minLon,maxLon)
NR-->>TS: nodes
TS->>ER: findEdgesInBoundingBox(minLat,maxLat,minLon,maxLon)
ER-->>TS: edges
TS->>Graph: buildGraph(filtered nodes, edges)
Graph-->>TS: graph
TS->>TS: find routes + addTransferWalkSteps
TS-->>User: TransportationResponse
sequenceDiagram
actor Caller
participant BIS as BusInfoService
participant RR as RouteRepository
Caller->>BIS: getBusInfo(stationName, busId, x, y)
BIS->>RR: findById(busId)
RR-->>BIS: route(Optional)
BIS->>BIS: createDummyBusInfo(...)
BIS-->>Caller: BusInfo(dummy)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (10)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (2)
32-34: 더미 데이터 경로 하드코딩 — 프로퍼티/플래그로 전환 권장서비스키 이슈가 해결되면 바로 원복할 수 있도록, 더미 경로 사용 여부를 설정(예: OpenDataProperties 또는 Spring 프로퍼티)로 분기하는 형태가 유지보수에 유리합니다. 현재는 항상 더미를 반환해 운영/스테이징 간 동작을 분리하기 어렵습니다.
향후 전환 계획이 있다면
openDataProperties.useDummyBusInfo()같은 플래그를 추가해 환경별로 제어하는 방향으로 가도 될까요?
103-121: 무작위 더미 응답 → 결정론적(시드 기반) 더미로 전환 권장Math.random() 사용으로 호출마다 응답이 달라져 캐싱, 테스트, UX에 불안정성이 생깁니다. 역/노선 식별자 기반 시드를 고정해 동일 입력에 대해 항상 같은 더미 응답이 나오도록 바꾸면 안정적입니다. 또한 Random/ThreadLocalRandom 사용이 성능/품질 면에서 낫습니다.
적용 예시(해당 메서드 범위 내 변경):
- List<Boolean> isLowFloor = new ArrayList<>(); - isLowFloor.add(Math.random() < 0.7); - isLowFloor.add(Math.random() < 0.5); - - Integer dispatchInterval = (int) (Math.random() * 15) + 1; + // 입력(stationName, busId) 기반 시드로 결정론적 더미 생성 + int seed = java.util.Objects.hash(stationName, busId); + java.util.Random rnd = new java.util.Random(seed); + + List<Boolean> isLowFloor = new ArrayList<>(); + isLowFloor.add(rnd.nextDouble() < 0.7); + isLowFloor.add(rnd.nextDouble() < 0.5); + + Integer dispatchInterval = rnd.nextInt(15) + 1; // 1~15분부가적으로 운영 로그 노이즈를 줄이려면 이 구간의 info 로그는 debug로 내리는 것도 고려해 주세요.
src/main/java/com/wayble/server/direction/repository/NodeRepository.java (1)
12-20: 바운딩 박스 쿼리 추가 LGTM — 컬럼 인덱스 고려 제안쿼리 자체는 명확합니다. 대용량에서 범위 필터가 빈번히 사용될 것이므로 Node.latitude/Node.longitude에 단일 혹은 복합 인덱스를 고려해 주세요. 공간 인덱스를 쓸 수 있는 DB면 그 활용도 검토할 가치가 있습니다.
src/main/java/com/wayble/server/direction/service/TransportationService.java (7)
42-44: SPATIAL_BUFFER_KM 하드코딩 → 설정화 권장지역/운영 환경에 따라 최적 버퍼가 다를 수 있으므로 프로퍼티로 추출해 튜닝 가능하게 해두면 운영 안정성이 올라갑니다.
예) application.yml에
direction.spatial-buffer-km: 15.0추가 후, @value 또는 설정 클래스로 주입.
124-131: OutOfMemoryError 직접 캐치는 지양OOME는 메모리 고갈로 JVM이 불안정해진 상태를 의미해, 복구 시도보다 빠른 실패가 권장됩니다. 일반 예외 로깅/처리로 한정하고 OOME는 전파시키는 편이 안전합니다.
- } catch (OutOfMemoryError e) { - log.error("Out of memory error in transportation route finding: {}", e.getMessage()); - throw new ApplicationException(PATH_NOT_FOUND); - } + } catch (RuntimeException e) { + log.error("Unexpected error in transportation route finding", e); + throw new ApplicationException(PATH_NOT_FOUND); + }
135-142: 경도 범위 계산 시 평균 위도 사용 제안(미세 정확도 개선)경도 → km 변환에 시작점 위도만 사용하는 대신, 시작/끝의 평균 위도를 사용하면 왜곡이 줄어듭니다. 기능엔 영향 없으나 정확도 개선에 도움이 됩니다.
- double minLon = Math.min(start.getLongitude(), end.getLongitude()) - SPATIAL_BUFFER_KM / (111.0 * Math.cos(Math.toRadians(start.getLatitude()))); - double maxLon = Math.max(start.getLongitude(), end.getLongitude()) + SPATIAL_BUFFER_KM / (111.0 * Math.cos(Math.toRadians(start.getLatitude()))); + double midLat = (start.getLatitude() + end.getLatitude()) / 2.0; + double lonKmPerDeg = 111.0 * Math.cos(Math.toRadians(midLat)); + double minLon = Math.min(start.getLongitude(), end.getLongitude()) - SPATIAL_BUFFER_KM / lonKmPerDeg; + double maxLon = Math.max(start.getLongitude(), end.getLongitude()) + SPATIAL_BUFFER_KM / lonKmPerDeg;
369-371: 중복 필터링 최소화 제안서비스 레벨에서 nodeIds로 한 번 더 거르는 것은 안전장치로 유효합니다. 다만 EdgeRepository를 AND 조건으로 개선(제안 반영 시)하면 이중 필터 비용을 줄일 수 있습니다.
381-385: 출발/도착 도보 연결 추가 호출 LGTM — 동일 ID의 임시 노드 중복 생성 주의현재 addOriginDestinationWalkConnections 내부에서 -1/-2 ID의 새 노드를 다시 만들고 graph.put으로 덮어씌웁니다. 기능상 문제는 없으나, startTmp/endTmp를 그대로 사용하면 객체 중복/혼동을 줄일 수 있습니다.
원하시면 해당 메서드에서 임시 노드 재생성 없이 startTmp/endTmp 재사용하는 미니 리팩터를 제안드리겠습니다.
736-773: addTransferWalkSteps 성능/가독성 양호 — 보행 스텝 최소 거리 컷오프 고려 가능현재 로직은 O(스텝 수)로 충분히 가볍습니다. 다만 너무 짧은 환승(예: 동일 승강장)에도 0m WALK가 삽입될 수 있으니, 0m는 생략하거나 최소 임계값 미만은 숨기는 UX 규칙 추가를 고려해볼 수 있습니다.
원하시면 임계값(예: 50m) 이하 WALK 스텝은 병합/제거하는 후처리 코드를 제안드리겠습니다.
775-808: calculateTransferWalkDistance: 동일 역명 다중 노드·후행 매칭 이슈 대비 제안역명이 동일한 노드가 여럿 존재할 수 있어 마지막 일치 노드로 덮어써질 수 있습니다. pathEdges를 한 번 순회하여 역명→최근 등장 노드 맵을 만들고, 여기서 조회하도록 바꾸면 의도한 매칭이 더 명확해집니다(성능 동일).
- Node fromNode = null; - Node toNode = null; - - for (Edge edge : pathEdges) { - if (edge.getStartNode() != null && edge.getStartNode().getStationName() != null && - edge.getStartNode().getStationName().equals(fromStation)) { - fromNode = edge.getStartNode(); - } - if (edge.getEndNode() != null && edge.getEndNode().getStationName() != null && - edge.getEndNode().getStationName().equals(fromStation)) { - fromNode = edge.getEndNode(); - } - if (edge.getStartNode() != null && edge.getStartNode().getStationName() != null && - edge.getStartNode().getStationName().equals(toStation)) { - toNode = edge.getStartNode(); - } - if (edge.getEndNode() != null && edge.getEndNode().getStationName() != null && - edge.getEndNode().getStationName().equals(toStation)) { - toNode = edge.getEndNode(); - } - } + java.util.Map<String, Node> lastSeen = new java.util.HashMap<>(); + for (Edge edge : pathEdges) { + if (edge.getStartNode() != null && edge.getStartNode().getStationName() != null) { + lastSeen.put(edge.getStartNode().getStationName(), edge.getStartNode()); + } + if (edge.getEndNode() != null && edge.getEndNode().getStationName() != null) { + lastSeen.put(edge.getEndNode().getStationName(), edge.getEndNode()); + } + } + Node fromNode = lastSeen.get(fromStation); + Node toNode = lastSeen.get(toStation);또는, 필요 시 양쪽에서 가장 가까운 조합을 찾는 방식으로 정밀도를 높일 수 있습니다(비용 증가).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java(2 hunks)src/main/java/com/wayble/server/direction/repository/NodeRepository.java(1 hunks)src/main/java/com/wayble/server/direction/service/BusInfoService.java(4 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/wayble/server/direction/repository/NodeRepository.java (1)
src/main/java/com/wayble/server/direction/repository/FacilityRepository.java (1)
FacilityRepository(12-17)
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java (2)
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (2)
Entity(8-47)createEdge(38-46)src/main/java/com/wayble/server/direction/repository/WheelchairInfoRepository.java (1)
WheelchairInfoRepository(11-15)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (3)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
BusInfo(39-43)src/main/java/com/wayble/server/direction/dto/response/BusInfo.java (2)
BusInfo(5-11)BusArrival(6-10)src/main/java/com/wayble/server/direction/external/opendata/dto/Arrival.java (1)
Arrival(3-7)
🔇 Additional comments (6)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (2)
35-100: 주석 처리된 실제 API 경로는 그대로 유지 OK서비스키 이슈 해소 전까지 주석 처리 유지 합리적으로 보입니다. 다만 장기화될 경우 feature flag로 분기하여 주석 블록은 제거하는 것이 가독성/충돌 예방에 좋습니다.
198-199: 빈 정류소 목록 경고 로그 추가 LGTM에지 케이스 로깅 보강 좋습니다. 디버깅에 유용할 것입니다.
src/main/java/com/wayble/server/direction/service/TransportationService.java (4)
93-113: 공간 필터링 기반 로드 도입 LGTM바운딩 박스 로드 + 최근접 노드 검증으로 입력 범위를 좁히는 접근이 OOM 완화에 실효적입니다. 박스 내 노드가 없을 때 PATH_NOT_FOUND 반환도 합리적입니다.
347-356: nodeIds 기반 사전 그래프 초기화 LGTM후속 엣지 필터링과 결합되어 KeyError, 불변 리스트 수정 예외 등의 잠재 이슈를 예방합니다.
431-454: 근접 노드 탐색의 1차 거칠은 필터 + 하버사인 정밀 체크 조합 LGTM성능·정확도 균형이 괜찮습니다. 극지방(코사인 0 근접) 케이스는 국내 서비스 특성상 무시 가능해 보입니다.
732-734: 환승 시 WALK 스텝 자동 삽입 달성PR 목적과 일치하며, 이후 UX 튜닝(예: 최소 보행거리 임계값 적용) 여지도 있습니다.
| @Query("SELECT DISTINCT e FROM Edge e " + | ||
| "JOIN FETCH e.startNode s " + | ||
| "JOIN FETCH e.endNode en " + | ||
| "LEFT JOIN FETCH e.route " + | ||
| "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) OR " + | ||
| "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)") | ||
| List<Edge> findEdgesInBoundingBox( | ||
| @Param("minLat") double minLat, | ||
| @Param("maxLat") double maxLat, | ||
| @Param("minLon") double minLon, | ||
| @Param("maxLon") double maxLon | ||
| ); |
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.
🛠️ Refactor suggestion
OR → AND로 변경해 공간 필터링 효과 증대(불필요한 엣지 로드 감소)
현재 조건은 시작/끝 중 하나만 박스 내면 매칭되어, 박스 외부 노드와 연결된 엣지도 대거 조회됩니다. 결국 서비스 레벨에서 nodeIds로 다시 거르지만, DB/네트워크/메모리 낭비가 커 OOM 완화 목적에 역행할 수 있습니다. 두 끝점 모두 박스 내인 엣지만 가져오도록 AND로 바꾸는 것을 권장합니다.
- "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) OR " +
- "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)")
+ "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) AND " +
+ "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)")추가로, Node(latitude, longitude) 컬럼 인덱싱 시 실효가 더 커집니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Query("SELECT DISTINCT e FROM Edge e " + | |
| "JOIN FETCH e.startNode s " + | |
| "JOIN FETCH e.endNode en " + | |
| "LEFT JOIN FETCH e.route " + | |
| "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) OR " + | |
| "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)") | |
| List<Edge> findEdgesInBoundingBox( | |
| @Param("minLat") double minLat, | |
| @Param("maxLat") double maxLat, | |
| @Param("minLon") double minLon, | |
| @Param("maxLon") double maxLon | |
| ); | |
| @Query("SELECT DISTINCT e FROM Edge e " + | |
| "JOIN FETCH e.startNode s " + | |
| "JOIN FETCH e.endNode en " + | |
| "LEFT JOIN FETCH e.route " + | |
| "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) AND " + | |
| "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)") | |
| List<Edge> findEdgesInBoundingBox( | |
| @Param("minLat") double minLat, | |
| @Param("maxLat") double maxLat, | |
| @Param("minLon") double minLon, | |
| @Param("maxLon") double maxLon | |
| ); |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/repository/EdgeRepository.java
around lines 19 to 30, the JPQL WHERE clause currently uses OR to match edges
whose start OR end node lies inside the bounding box, causing many edges with
one endpoint outside the box to be loaded; change the OR to AND so only edges
whose startNode AND endNode are both inside the box are returned, and update any
tests accordingly; additionally, add or ensure database indexes on Node.latitude
and Node.longitude (or a composite spatial index if supported) to improve filter
performance and reduce memory/IO overhead.
seung-in-Yoo
left a 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.
오 코드가 되게 알아보기 편한거같아요 수고하셨습니다~!!
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit