♻️ refactor: 미션 목록 조회 N+1 제거 및 성능 측정 정리 - #120
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe mission-list API now retrieves today’s completions in one batch query, maps the latest completion per mission, and preserves response status fields. Integration measurements, k6 scenarios, execution guidance, and before-and-after results were added. ChangesMission-list completion batching
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MissionQueryService
participant MissionCompletionRepository
participant MissionListResponse
Client->>MissionQueryService: Request mission list
MissionQueryService->>MissionCompletionRepository: Query today's completions for selected mission IDs
MissionCompletionRepository-->>MissionQueryService: Return ordered completion records
MissionQueryService->>MissionListResponse: Map latest completion into mission summaries
MissionListResponse-->>Client: Return mission list response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 6
🧹 Nitpick comments (1)
scripts/loadtest/mission-list.k6.js (1)
72-77: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScope thresholds by scenario when multiple scenarios run together.
The default configuration enables multiple scenarios, but these thresholds aggregate all requests. A fast scenario can dilute a slow
spike_loadscenario and allow the combined p95 to pass. Add scenario-scoped thresholds or require one enabled scenario for each gated measurement. k6 supports tag-filtered thresholds. (grafana.com)🤖 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 `@scripts/loadtest/mission-list.k6.js` around lines 72 - 77, Scope the http_req_failed and http_req_duration thresholds in options to the relevant scenario using k6 tag-filtered metric thresholds, especially ensuring spike_load is evaluated independently. Preserve the existing threshold limits while preventing results from other concurrently enabled scenarios from diluting gated measurements.Source: 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 `@docs/performance/mission-list-n-plus-one-measurement.md`:
- Line 250: Update the performance-results statement around the mission
completion batch lookup to remove the claim that response-time distributions
stabilized. State only that SQL query counts decreased, while latency results
were mixed across load levels and improved only in the 100 VU comparison.
- Around line 122-128: Update
docs/performance/mission-list-n-plus-one-measurement.md at lines 122-128 to
document the reproducible spike runs: two SPIKE_VUS=50 runs, three SPIKE_VUS=100
runs, and two SPIKE_VUS=200 runs; alternatively, remove unsupported portfolio
results from docs/portfolio/mission-list-n-plus-one-optimization.md at lines
43-51. Keep the measurement guide and recorded portfolio results aligned.
In `@scripts/loadtest/mission-list.k6.js`:
- Around line 11-15: Update constantVuScenario to validate the resolved vus
value as a finite, non-negative integer before creating the scenario; reject
malformed and fractional values with an error that identifies vusEnvName, while
preserving 0 as the disable value that returns null.
- Around line 91-100: Update the load-test options thresholds to include a
checks threshold requiring a 100% success rate, alongside the existing
http_req_failed and http_req_duration thresholds. This must make failures
recorded by the response check in the check call cause the test to fail.
In `@src/main/kotlin/com/zerost/api/mission/application/MissionQueryService.kt`:
- Around line 125-133: Update getMission to use the half-open range query
findTopByUserIdAndMissionIdAndSubmittedAtGreaterThanEqualAndSubmittedAtLessThanOrderBySubmittedAtDesc,
matching the mission-list endpoint’s inclusive-start/exclusive-end behavior. Add
a regression test covering a completion timestamped at
today.plusDays(1).atStartOfDay() and verify it is excluded consistently.
In
`@src/test/kotlin/com/zerost/api/mission/application/MissionQueryServiceMeasurementTest.kt`:
- Around line 121-122: Update the assertions in
MissionQueryServiceMeasurementTest to enforce the documented optimized SQL
limit: require statementCount to be at most four instead of merely greater than
zero, while preserving the avgElapsedMs validation.
---
Nitpick comments:
In `@scripts/loadtest/mission-list.k6.js`:
- Around line 72-77: Scope the http_req_failed and http_req_duration thresholds
in options to the relevant scenario using k6 tag-filtered metric thresholds,
especially ensuring spike_load is evaluated independently. Preserve the existing
threshold limits while preventing results from other concurrently enabled
scenarios from diluting gated measurements.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 811daafc-0fb3-4877-9abd-2b5f656c82dd
⛔ Files ignored due to path filters (14)
docs/assets/mission-list-after-k6-100vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-k6-10vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-k6-1vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-k6-200vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-k6-30vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-k6-50vu.pngis excluded by!**/*.pngdocs/assets/mission-list-after-test-and-sql.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-100vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-10vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-1vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-200vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-30vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-k6-50vu.pngis excluded by!**/*.pngdocs/assets/mission-list-before-test-and-sql.pngis excluded by!**/*.png
📒 Files selected for processing (7)
docs/performance/mission-list-n-plus-one-measurement.mddocs/portfolio/mission-list-n-plus-one-optimization.mdscripts/loadtest/mission-list.k6.jssrc/main/kotlin/com/zerost/api/mission/application/MissionQueryService.ktsrc/main/kotlin/com/zerost/api/mission/domain/MissionCompletionRepository.ktsrc/test/kotlin/com/zerost/api/mission/application/MissionQueryServiceMeasurementTest.ktsrc/test/kotlin/com/zerost/api/mission/application/MissionQueryServiceTest.kt
Comment on lines
+122
to
+128
| 포트폴리오 용도로는 아래 3개 시나리오만 우선 비교해도 충분합니다. | ||
|
|
||
| - `1 VU` | ||
| - `10 VU` | ||
| - `30 VU` | ||
|
|
||
| `50 VU`는 스파이크 참고 자료로 선택적으로 사용하면 됩니다. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant docs lines =="
for f in docs/performance/mission-list-n-plus-one-measurement.md docs/portfolio/mission-list-n-plus-one-optimization.md; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,180p'
else
echo "missing $ f"
fi
done
echo "== spike VUS references =="
rg -n "SPIKE_VUS|100 VU|200 VU|spike|스파이크|VU" docs/performance docs/portfolio -S || true
echo "== related load script candidates =="
fd -a -i 'load|test|k6|artillery|ab|wrk|siege|benchmark' . | sed 's#^\./##' | head -100
rg -n "n_plus_one|mission.*list|스파이크|SPIKE|v-us|vu" . -g '!node_modules' -g '!dist' -g '!build' --glob '!*.md' || trueRepository: team-0st/BE
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant docs lines =="
for f in docs/performance/mission-list-n-plus-one-measurement.md docs/portfolio/mission-list-n-plus-one-optimization.md; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,180p'
else
echo "missing $f"
fi
done
echo "== spike VUS references =="
rg -n "SPIKE_VUS|100 VU|200 VU|spike|스파이크|VU" docs/performance docs/portfolio -S || true
echo "== related load script candidates =="
fd -a -i 'load|test|k6|artillery|ab|wrk|siege|benchmark' . | sed 's#^\./##' | head -100
rg -n "n_plus_one|mission.*list|스파이크|SPIKE|v-us|vu" . -g '!node_modules' -g '!dist' -g '!build' --glob '!*.md' || trueRepository: team-0st/BE
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for f in [
Path("docs/performance/mission-list-n-plus-one-measurement.md"),
Path("docs/portfolio/mission-list-n-plus-one-optimization.md"),
]:
print(f"--- {f} ---")
if f.exists():
text = f.read_text(encoding="utf-8")
for i, line in enumerate(text.splitlines(), 1):
if 1 <= i <= 180:
print(f"{i:4}: {line}")
else:
print("missing")
PY
echo "== spike VUS references =="
python3 - <<'PY'
from pathlib import Path
terms = ["SPIKE_VUS","100 VU","200 VU","spike","스파이크","VU"]
roots = [Path("docs/performance"), Path("docs/portfolio")]
for root in roots:
for p in root.rglob("*"):
if p.is_file() and p.suffix in [".md",".txt",".yml",".yaml",".json",".sh",".js",".ts",".py"]:
try:
text = p.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
for term in terms:
if term in text:
lines = [i for i, line in enumerate(text.splitlines(), 1) if term in line]
print(f"{p}:{','.join(str(i) for i in lines[:10])}: {term}")
break
PY
echo "== related load script candidates =="
python3 - <<'PY'
from pathlib import Path
import re
terms = ["load","test","k6","artillery","ab","wrk","siege","benchmark","n_plus_one","mission.*list","스파이크","SPIKE","v-us","vu"]
for root in Path(".").iterdir():
if root.name in {".git","node_modules","dist","build"} or root.is_file():
continue
for p in root.rglob("*"):
if p.is_file():
filename = p.name.lower()
if any(x in filename for x in ["load","test","k6","artillery","ab","wrk","siege","benchmark"]) or p.suffix in {".sh",".js",".ts",".py",".mjs",".cjs"}:
try:
text = p.read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
for term in terms:
if re.search(term, text, re.I):
print(f"{p}")
break
PYRepository: team-0st/BE
Length of output: 17754
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/loadtest/mission-list.k6.js")
if p.exists():
text = p.read_text(encoding="utf-8")
lines = text.splitlines()
for term in ["SMALL_VUS", "MEDIUM_VUS", "SPIKE_VUS", "threshold"]:
for i, line in enumerate(lines, 1):
if term in line:
start = max(1, i-6)
end = min(len(lines), i+6)
print(f"\n--- {term} at line {i} ---")
for n in range(start, end+1):
print(f"{n:4}: {lines[n-1]}")
else:
print("missing scripts/loadtest/mission-list.k6.js")
PY
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/loadtest/mission-list.k6.js")
if p.exists():
text = p.read_text(encoding="utf-8")
import re
for m in re.finditer(r'export function runScenario\([^)]*\)\s*\{', text):
idx = text.count("\n", 0, m.start()) + 1
body = text[m.start():]
end_br = body.find("}")
while body.count("{") - body.count("}") > 1 and end_br != -1:
end_br = body.find("}", end_br + 1)
print(f"--- runScenario starts around line {idx} ---")
print("\n".join(body[:2000].splitlines()))
PY
python3 - <<'PY'
from pathlib import Path
files = [
Path("docs/performance/mission-list-n-plus-one-measurement.md"),
Path("docs/portfolio/mission-list-n-plus-one-optimization.md"),
Path("scripts/loadtest/mission-list.k6.js"),
]
for f in files:
if f.exists():
text = f.read_text(encoding="utf-8")
else:
continue
print(f"\n== {f} ==")
for pattern in [
r'for i in 1 2 3; do',
r'-e SMALL_VUS=.+',
r'-e MEDIUM_VUS=.+',
r'-e SPIKE_VUS=.+',
r'100 VU spike|200 VU spike',
]:
print(f"\n--- pattern {pattern} ---")
for i, line in enumerate(text.splitlines(), 1):
if re.search(pattern, line):
print(f"{i}: {line}")
PYRepository: team-0st/BE
Length of output: 1861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
docs_performance = Path("docs/performance/mission-list-n-plus-one-measurement.md")
docs_portfolio = Path("docs/portfolio/mission-list-n-plus-one-optimization.md")
script = Path("scripts/loadtest/mission-list.k6.js")
for f in [docs_performance, docs_portfolio, script]:
text = f.read_text(encoding="utf-8")
print(f"\n== {f} ==")
for pattern in [
r'for i in 1 2 3; do',
r'spike_load\s*,\s*SPIKE_VUS\s*,\s*50',
r'-e SMALL_VUS=(0|1)',
r'-e MEDIUM_VUS=(0|30)',
r'-e SPIKE_VUS=(50|100|200)',
r'100 VU spike',
r'200 VU spike',
r'100 VU',
r'200 VU',
]:
print(f"\n--- pattern {pattern} ---")
for i, line in enumerate(text.splitlines(), 1):
if re.search(pattern, line):
print(f"{i}: {line}")
PYRepository: team-0st/BE
Length of output: 2589
Align the measurement guide with the recorded spike runs.
docs/portfolio/mission-list-n-plus-one-optimization.md documents 100 VU spike and 200 VU spike results, but the measurement guide only covers up to 50 VU. Add the exact spikes: two SPIKE_VUS=50 runs, three SPIKE_VUS=100 runs, and two SPIKE_VUS=200 runs, or remove the portfolio results that cannot be reproduced from these docs.
📍 Affects 2 files
docs/performance/mission-list-n-plus-one-measurement.md#L122-L128(this comment)docs/portfolio/mission-list-n-plus-one-optimization.md#L43-L51
🤖 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 `@docs/performance/mission-list-n-plus-one-measurement.md` around lines 122 -
128, Update docs/performance/mission-list-n-plus-one-measurement.md at lines
122-128 to document the reproducible spike runs: two SPIKE_VUS=50 runs, three
SPIKE_VUS=100 runs, and two SPIKE_VUS=200 runs; alternatively, remove
unsupported portfolio results from
docs/portfolio/mission-list-n-plus-one-optimization.md at lines 43-51. Keep the
measurement guide and recorded portfolio results aligned.
Comment on lines
+125
to
+133
| return missionCompletionRepository | ||
| .findAllByUserIdAndMissionIdInAndSubmittedAtGreaterThanEqualAndSubmittedAtLessThanOrderByMissionIdAscSubmittedAtDesc( | ||
| userId = userId, | ||
| missionId = requireNotNull(id), | ||
| missionIds = missionIds, | ||
| start = todayRange.first, | ||
| end = todayRange.second, | ||
| ) | ||
| .groupBy { requireNotNull(it.mission.id) } | ||
| .mapValues { (_, completions) -> completions.first() } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'MissionQueryService\.kt|Mission.*Repository|Mission.*Controller|Mission.*Service' || true
echo
echo "== target file outline/lines =="
wc -l src/main/kotlin/com/zerost/api/mission/application/MissionQueryService.kt 2>/dev/null || true
sed -n '1,180p' src/main/kotlin/com/zerost/api/mission/application/MissionQueryService.kt 2>/dev/null || true
echo
echo "== repository query methods =="
rg -n "findAllByUserIdAndMissionIdIn|missionCompletionRepository|Between|GreaterThanEqual|LessThan|todayRange|atStartOfDay|now" src/main/kotlin -SRepository: team-0st/BE
Length of output: 21419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository query methods =="
rg -n "findAllByUserIdAndMissionIdIn|missionCompletionRepository|Between|GreaterThanEqual|LessThan|todayRange|atStartOfDay|now" src/main/kotlin -SRepository: team-0st/BE
Length of output: 10214
🌐 Web query:
Spring Data JPA Between query method includes bounds SQL between documentation
💡 Result:
In Spring Data JPA, the Between query method keyword translates directly to the standard SQL BETWEEN operator [1][2][3]. According to the official Spring Data JPA documentation, a query method defined with the Between keyword (e.g., findByStartDateBetween) is translated into the SQL clause:... where x.startDate between?1 and?2 [1][2][3]. Because it maps to the standard SQL BETWEEN operator, the bounds are inclusive [1][2][3]. This means the query will retrieve records where the field value is greater than or equal to the first parameter and less than or equal to the second parameter.
Citations:
- 1: https://docs.spring.io/spring-data/jpa/reference/4.0/jpa/query-methods.html
- 2: https://docs.spring.io/spring-data/jpa/reference/jpa/query-methods.html
- 3: https://docs.spring.io/spring-data/jpa/reference/4.0-SNAPSHOT/jpa/query-methods.html
Use the same half-open day range in both endpoints.
getMission uses SubmittedAtBetween, which includes todayRange.second. A completion at midnight can appear in the detail response but not in the mission-list response. Use findTopByUserIdAndMissionIdAndSubmittedAtGreaterThanEqualAndSubmittedAtLessThanOrderBySubmittedAtDesc and add a regression test for a completion at today.plusDays(1).atStartOfDay().
🤖 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 `@src/main/kotlin/com/zerost/api/mission/application/MissionQueryService.kt`
around lines 125 - 133, Update getMission to use the half-open range query
findTopByUserIdAndMissionIdAndSubmittedAtGreaterThanEqualAndSubmittedAtLessThanOrderBySubmittedAtDesc,
matching the mission-list endpoint’s inclusive-start/exclusive-end behavior. Add
a regression test covering a completion timestamped at
today.plusDays(1).atStartOfDay() and verify it is excluded consistently.
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
1 participant
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.
#️⃣ Issue Number
📝 요약(Summary)
MissionCompletion조회를 배치 조회 방식으로 정리해 N+1 패턴을 제거했습니다.missionId기준으로 매핑해 기존 응답 구조와 상태 계산 의미를 유지하도록 변경했습니다.📝 리뷰 요청사항
MissionCompletion은 하루 안에도 여러 제출이 가능해 단순 fetch join 대신 "오늘 범위 조회 → missionId별 최신 1건 선택" 방식으로 정리했습니다.💻 테스트 결과
./gradlew test --tests 'com.zerost.api.mission.application.MissionQueryServiceTest'통과./gradlew test --tests 'com.zerost.api.mission.application.MissionQueryServiceMeasurementTest'통과k6로1 / 10 / 30 / 50 / 100 / 200 VU시나리오 측정 및 비교 완료📌 포인트
7 -> 4로 줄여 조회 구조를 단순화한 점을 수치로 확인했습니다.100 VU구간에서는 평균 응답 시간과 p95가 소폭 개선됐고,200 VU구간은 dev 환경 편차도 함께 관찰했습니다.✍️ 회고
Summary by CodeRabbit
Performance Improvements
Testing
Documentation