[Perf] Worker 수·실행 Slot별 전체 인덱싱 수평 확장 Benchmark 추가 - #140
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughWorker 수와 실행 슬롯 조합을 측정하는 전용 통합 Benchmark를 추가했습니다. 독립 Worker Context에서 실제 인덱싱 Pipeline을 실행하고, 처리량·지연·Worker 분포·정합성을 검증한 뒤 JSON과 결과 문서에 기록합니다. ChangesWorker 수평 확장 Benchmark
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BenchmarkTask
participant WorkerHorizontalScalingBenchmark
participant WorkerContext
participant PostgreSQL
participant MinIO
participant BGEM3
BenchmarkTask->>WorkerHorizontalScalingBenchmark: 전용 테스트 실행
WorkerHorizontalScalingBenchmark->>WorkerContext: Worker Context 기동
WorkerHorizontalScalingBenchmark->>MinIO: 문서 병렬 업로드
WorkerContext->>PostgreSQL: Job Claim 및 상태 갱신
WorkerContext->>MinIO: 문서 조회
WorkerContext->>BGEM3: Embedding 생성
WorkerContext->>PostgreSQL: Chunk·Embedding·Vector 저장
WorkerHorizontalScalingBenchmark->>PostgreSQL: 정합성과 Worker 분포 검증
WorkerHorizontalScalingBenchmark->>BenchmarkTask: JSON 결과 저장
Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java (3)
777-783: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
count와queryInteger가 완전히 같은 구현입니다.두 메서드 모두
jdbcTemplate.queryForObject(sql, Integer.class, arguments)를 호출합니다. 반환 타입도int로 동일합니다. 이름만 다릅니다. 호출 지점에서 어느 쪽을 써야 하는지 판단 기준이 없고, 실제로 Line 467과 Line 471은 같은 성격의 조회에 서로 다른 이름을 사용합니다.하나로 합치는 편이 명확합니다. 다만 Line 502-509의
vector_dims조회처럼 "개수"가 아닌 값을 읽는 지점이 있으므로,queryInteger하나만 남기는 방향을 권장합니다.🤖 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/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java` around lines 777 - 783, Remove the duplicate count method and retain queryInteger as the single integer-query helper, updating all count call sites to use queryInteger while preserving the existing vector_dims and other non-count lookups.
626-661: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value두 초기화 메서드의 TRUNCATE 대상 목록이 중복됩니다.
resetJobState와resetAllBenchmarkState는 8개 테이블 목록이 동일합니다. 차이는worker_nodes포함 여부 하나뿐입니다. 나중에 테이블이 추가되면 두 곳을 모두 고쳐야 하고, 한쪽을 빠뜨리면 Profile 간 데이터가 남습니다.공통 목록을 상수로 추출하고
worker_nodes만 조건부로 덧붙이는 방식을 검토해 주세요.🤖 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/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java` around lines 626 - 661, Extract the shared eight-table TRUNCATE list from resetJobState and resetAllBenchmarkState into a reusable constant or helper. Build each statement from that shared list, adding worker_nodes only for resetAllBenchmarkState, while preserving the existing reset behavior and ordering.
452-560: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffJob 정합성 검증이 문서 하나당 개별 쿼리 10건 이상을 실행합니다.
Line 463의 루프는 문서마다 상태, 재시도 수, Attempt 수, Chunk 수, Embedding 수, Vector 차원, Event 목록을 각각 별도 쿼리로 조회합니다. 기본 설정에서 16문서 × 5 Profile × 2 반복이면 검증 쿼리만 1,600건을 넘습니다.
이 구간은 처리량 측정 창(Line 363의
profileCompletedAt) 밖이므로 측정값은 왜곡되지 않습니다. 다만 전체 실행 시간이 늘어납니다.document-count를 크게 올려 실행할 계획이 있으면 문서 목록 단위 집계 쿼리로 묶는 방안을 검토해 주세요. 현재 기본 규모에서는 그대로 두어도 무방합니다.🤖 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/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java` around lines 452 - 560, assertProfileInvariants의 문서별 검증 쿼리를 문서 목록 단위 집계 쿼리로 통합해 실행 횟수를 줄이세요. 상태·retry·attempt·chunk·embedding·vector 차원·이벤트 검증에 필요한 결과를 여러 업로드 ID에 대해 한 번에 조회하고, 기존의 문서별 불변식과 실패 조건은 동일하게 유지하세요. 기본 규모 동작과 Profile 결과 생성은 변경하지 마세요.src/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingStatisticsTest.java (1)
17-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win기본값 폴백과 경계 입력 검증을 추가해 주세요.
현재 테스트는 정상 파싱, 중복, Baseline 누락, 계산 세 경로만 다룹니다. 다음 경로는 검증되지 않습니다.
rawValue가null또는 공백일 때defaults를 그대로 반환하는 경로. 이 경로는 System Property를 지정하지 않는 기본 실행 경로입니다.- 형식 오류 입력(
"1","1x2x3") 거부.WorkerProfile의workerCount·slotsPerWorker0 이하 거부.speedup의 Baseline 0 이하 거부와scalingEfficiency의totalSlots0 이하 거부.🧪 추가 테스트 예시
`@Test` `@DisplayName`("Profile 입력이 비어 있으면 기본 Profile 목록을 사용한다") void fallsBackToDefaultProfiles() { List<WorkerProfile> defaults = List.of(new WorkerProfile(1, 1), new WorkerProfile(2, 2)); assertThat(WorkerHorizontalScalingStatistics.parseProfiles(null, defaults)) .isEqualTo(defaults); assertThat(WorkerHorizontalScalingStatistics.parseProfiles(" ", defaults)) .isEqualTo(defaults); } `@Test` `@DisplayName`("형식과 값 경계를 벗어난 입력을 거부한다") void rejectsMalformedAndNonPositiveInputs() { assertThatThrownBy(() -> WorkerHorizontalScalingStatistics.parseProfiles("1", List.of())) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> WorkerHorizontalScalingStatistics.parseProfiles("1x2x3", List.of())) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> WorkerHorizontalScalingStatistics.parseProfiles("0x1", List.of())) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> WorkerHorizontalScalingStatistics.speedup(0.0, 1.0)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> WorkerHorizontalScalingStatistics.scalingEfficiency(1.0, 0)) .isInstanceOf(IllegalArgumentException.class); }🤖 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/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingStatisticsTest.java` around lines 17 - 59, Update WorkerHorizontalScalingStatistics.parseProfiles to return the exact defaults list when rawValue is null or blank, reject malformed profile formats and non-positive workerCount or slotsPerWorker values, and preserve existing duplicate/baseline validation. Add boundary validation in speedup for non-positive baseline throughput and in scalingEfficiency for non-positive totalSlots, throwing IllegalArgumentException.Source: Path instructions
🤖 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/design/gimin-`#138-worker-horizontal-scaling-benchmark.md:
- Around line 106-119: Update the System Property list in the benchmark
documentation to include worker.horizontal.scaling.document-characters and
worker.horizontal.scaling.status-polling-ms. Extend the profile requirements to
state that the profile list must include 1x1 as the speedup baseline, while
preserving the existing validation requirements.
In `@docs/test-results/gimin-`#138-worker-horizontal-scaling-benchmark.md:
- Around line 150-154: Update the conclusion in section 10 so “균등 분담” is
explicitly framed as an observation from this benchmark run, not a guaranteed
behavior or invariant. Preserve the existing claims about participation,
indexing, and vector invariants, while avoiding wording that implies the
benchmark enforces balanced Worker distribution.
In
`@src/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java`:
- Around line 314-333: Update the try/catch cleanup around WorkerCluster
creation so assertion failures from awaitCondition and the assertThat checks
also trigger closeContexts(contexts). Catch the appropriate broader throwable
type while preserving propagation of the original failure and existing
InterruptedException behavior.
- Around line 171-184: Update cleanUpInfrastructure so the TEST_SCHEMA drop
executes in a finally block after minioBucket cleanup, ensuring JDBC schema
removal occurs even when minioBucket.close() throws. Preserve the existing
currentCluster cleanup and bucket cleanup behavior.
- Around line 210-215: Update the cleanup flow surrounding awaitWorkersStopped
in the finally block of the benchmark method to preserve any exception from the
try body when shutdown verification also fails. Catch the pending cleanup
AssertionError and attach it to the original failure via addSuppressed, or
otherwise ensure cleanup failure cannot replace the primary exception; retain
the existing close and worker-stop behavior.
- Around line 765-775: Update awaitCondition to accept a Supplier<String> and
evaluate it only when the timeout AssertionError is created, then add the
java.util.function.Supplier import. Convert the three affected call sites around
the job/worker snapshot messages to lambdas so jobSnapshot(uploads) and
workerSnapshot(cluster) run at timeout rather than before waiting; also wrap the
constant message call around lines 314-317 in a lambda.
---
Nitpick comments:
In
`@src/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.java`:
- Around line 777-783: Remove the duplicate count method and retain queryInteger
as the single integer-query helper, updating all count call sites to use
queryInteger while preserving the existing vector_dims and other non-count
lookups.
- Around line 626-661: Extract the shared eight-table TRUNCATE list from
resetJobState and resetAllBenchmarkState into a reusable constant or helper.
Build each statement from that shared list, adding worker_nodes only for
resetAllBenchmarkState, while preserving the existing reset behavior and
ordering.
- Around line 452-560: assertProfileInvariants의 문서별 검증 쿼리를 문서 목록 단위 집계 쿼리로 통합해
실행 횟수를 줄이세요. 상태·retry·attempt·chunk·embedding·vector 차원·이벤트 검증에 필요한 결과를 여러 업로드
ID에 대해 한 번에 조회하고, 기존의 문서별 불변식과 실패 조건은 동일하게 유지하세요. 기본 규모 동작과 Profile 결과 생성은 변경하지
마세요.
In
`@src/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingStatisticsTest.java`:
- Around line 17-59: Update WorkerHorizontalScalingStatistics.parseProfiles to
return the exact defaults list when rawValue is null or blank, reject malformed
profile formats and non-positive workerCount or slotsPerWorker values, and
preserve existing duplicate/baseline validation. Add boundary validation in
speedup for non-positive baseline throughput and in scalingEfficiency for
non-positive totalSlots, throwing IllegalArgumentException.
🪄 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: 62c14357-46ac-499a-a48a-f07389a5757c
📒 Files selected for processing (6)
build.gradledocs/design/gimin-#138-worker-horizontal-scaling-benchmark.mddocs/test-results/gimin-#138-worker-horizontal-scaling-benchmark.mdsrc/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingBenchmark.javasrc/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingStatistics.javasrc/test/java/com/opensource/docgrid/e2e/WorkerHorizontalScalingStatisticsTest.java
🔍️ 작업 내용
vector(1024)정합성을 Profile마다 함께 검증합니다.✨ 상세 설명
Profile
w1-s1w1-s2w2-s1w2-s2w4-s2로컬 정식 측정 결과
16문서, 문서당 8 Chunk·Embedding, Profile별 2회 반복 중앙값입니다.
w1-s1w1-s2w2-s1w2-s2w4-s2다중 Worker는 본 측정에서 실제 Job을 균등 분담했습니다. 전체 Slot 2개 이후 처리량은 약 21.6문서/분에서 포화되고 처리 p95가 증가해, 현재 단일 Host CPU BGE-M3가 공유 병목임을 확인했습니다.
실행 경계
일반
test에서는worker-horizontal-scalingTag를 제외하며 전용 Task는 단일 Fork와 Build Cache 비활성화로 실행됩니다.✅ 검증
./gradlew test --tests com.opensource.docgrid.e2e.WorkerHorizontalScalingStatisticsTestw1-s1,w2-s1실제 인프라 SmokeDB_SSLMODE=disable ./gradlew workerHorizontalScalingTestBUILD SUCCESSFUL in 8m 12s./gradlew test./gradlew build상세 결과:
docs/test-results/gimin-#138-worker-horizontal-scaling-benchmark.md🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
새로운 기능
문서
테스트