[refactor] #72 N+1 문제 해결을 위한 매칭 후보자 탐색 쿼리 최적화#113
Conversation
|
""" WalkthroughThe changes optimize the participant matching query by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant MatchingService
participant MatchingRepository
participant Database
MatchingService->>MatchingRepository: findMatchingCandidates(..., PageRequest.of(0,1))
MatchingRepository->>Database: Query with JOIN FETCH user, LEFT JOIN Matching, GROUP BY, ORDER BY COUNT()
Database-->>MatchingRepository: List<Participant> with User eagerly loaded
MatchingRepository-->>MatchingService: List<Participant>
MatchingService->>MatchingService: Stream list, findFirst candidate
MatchingService->>MatchingService: saveMatching(Optional.ofNullable(candidate))
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (10)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results44 tests 44 ✅ 1s ⏱️ Results for commit 0f645a5. ♻️ This comment has been updated with latest results. |
![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 (1)
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java (1)
16-18: Consider addingDISTINCTto avoid accidental duplicates after theJOIN FETCH.
p.useris probably a@ManyToOne, so in theory a duplicate row cannot be produced.
However, if the mapping is ever switched to@OneToMany/@ManyToManyin the future (or furtherJOIN FETCHes are appended), the current query would suddenly start returning duplicates and break the single-row expectation of the method.
A defensive change keeps the query stable:-SELECT p FROM Participant p -JOIN FETCH p.user +SELECT DISTINCT p FROM Participant p +JOIN FETCH p.user
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java(1 hunks)src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java(1 hunks)src/test/java/org/festimate/team/domain/festival/entity/FestivalTest.java(1 hunks)src/test/java/org/festimate/team/domain/festival/service/impl/FestivalServiceImplTest.java(1 hunks)src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java(1 hunks)
🔇 Additional comments (5)
src/test/java/org/festimate/team/domain/festival/service/impl/FestivalServiceImplTest.java (1)
21-21: Improved visibility modifier for test class.Changing the visibility from
publicto package-private follows best practices for test classes, as they are typically only used within their package.src/test/java/org/festimate/team/domain/festival/entity/FestivalTest.java (1)
14-14: Improved visibility modifier for test class.Changing the visibility from
publicto package-private follows best practices for test classes, as they are typically only used within their package.src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java (1)
33-33: Improved visibility modifier for test class.Changing the visibility from
publicto package-private follows best practices for test classes, as they are typically only used within their package.src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java (1)
50-53: Improved variable naming and Optional handling.The renaming from
targetParticipantOptionaltotargetOptionalmakes the code more concise, and the explicit unwrapping withorElse(null)followed by rewrapping withOptional.ofNullable()makes the flow clearer.This change aligns with the PR objective of optimizing the matching candidate search logic by making the code more explicit about when and how optional values are accessed.
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java (1)
22-27: Previous matches are filtered only in one direction – is that intentional?The sub-query excludes candidates when the caller was
applicantand the candidate wastarget.
If a completed matching exists in the opposite direction (caller was the target), the candidate is still returned.
If “matched once means never match again” is the rule, this condition should be symmetric:-AND p.participantId NOT IN ( - SELECT m.targetParticipant.participantId - FROM Matching m - WHERE m.applicantParticipant.participantId = :participantId - AND m.status = 'COMPLETED' -) +AND p.participantId NOT IN ( + SELECT CASE + WHEN m.applicantParticipant.participantId = :participantId + THEN m.targetParticipant.participantId + ELSE m.applicantParticipant.participantId + END + FROM Matching m + WHERE (m.applicantParticipant.participantId = :participantId + OR m.targetParticipant.participantId = :participantId) + AND m.status = 'COMPLETED' +)Please confirm the business rule to avoid repeat pairings in the opposite direction.
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java (2)
17-31: Consider adding a comment explaining the query optimization.The query optimization is well-implemented, but it would be beneficial to add a comment explaining the approach and its performance benefits for future maintainers.
@Query(""" + // Optimized query to solve N+1 problem by eagerly loading users + // and ordering participants by their matching count in ascending order SELECT p FROM Participant p JOIN FETCH p.user LEFT JOIN Matching m1 ON (m1.applicantParticipant = p OR m1.targetParticipant = p) WHERE p.festival.festivalId = :festivalId AND p.typeResult = :typeResult AND p.user.gender != :gender AND p.participantId != :participantId AND p.participantId NOT IN ( SELECT m.targetParticipant.participantId FROM Matching m WHERE m.applicantParticipant.participantId = :participantId AND m.status = 'COMPLETED' ) GROUP BY p ORDER BY COUNT(m1) ASC """)
41-41: Consider consistent spacing.There's an extra blank line that could be removed to maintain consistent spacing between repository methods.
); - @Query("""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java(1 hunks)src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java(3 hunks)src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java
- src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java
🔇 Additional comments (5)
src/main/java/org/festimate/team/domain/matching/repository/MatchingRepository.java (5)
7-7: Appropriate addition of Pageable import.The addition of Pageable supports the new pagination capability in the repository method, which is consistent with the optimizations mentioned in the PR objectives.
17-19: Great optimization with JOIN FETCH and improved join strategy!The
JOIN FETCH p.usercorrectly addresses the N+1 query problem by eagerly loading the User entities associated with Participants. TheLEFT JOINwith the OR condition is an efficient approach for counting relationships where the participant appears on either side.
25-26: Clean subquery formulation.The subquery is well-structured and correctly identifies participants that have already been matched with the current participant.
30-31: Effective replacement of subquery with GROUP BY and ORDER BY.This change replaces the previous subquery-based minimum count filter with a more efficient approach using GROUP BY and ORDER BY COUNT, which should perform better, especially with larger datasets.
33-39: Repository method signature properly updated for pagination.The method has been appropriately renamed from
findMatchingCandidatetofindMatchingCandidatesand now returns aList<Participant>with pagination support. This addresses the potentialNonUniqueResultExceptionmentioned in previous reviews while maintaining the performance benefits.
# Conflicts: # src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java # src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java
1 participant
📌 PR 제목
[refactor] #72 N+1 문제 해결을 위한 매칭 후보자 탐색 쿼리 최적화
📌 PR 내용
매칭 요청 시 사용되는 후보자 탐색 쿼리에서 다음과 같은 구조적/성능적 문제를 해결했습니다:
🛠 작업 내용
✅ 1. 후보자 탐색 쿼리 리팩터링
✅ 2. 서비스 로직/테스트 코드 대응
✅ 기대 효과
🔍 관련 이슈
Closes #72
📸 스크린샷 (Optional)
N/A
📚 레퍼런스 (Optional)
🐌 JPA에서 Lazy 로딩, 정말 느린 걸까?
Summary by CodeRabbit