[Test] 실제 MinIO 업로드 동시성 검증 - #30
Merged
Merged
Conversation
🚥 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: 2
🤖 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
`@src/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java`:
- Around line 126-140: Update both ExecutorService cleanup blocks in
MinioUploadConcurrencyIntegrationTest.java (lines 126-140 and 188-201): cancel
the submitted futures during cleanup, call shutdownNow(), and awaitTermination
before allowing teardown to continue, handling interruption while preserving the
interrupt status.
- Around line 107-113: Update tearDown() so cleanupBucket() and
storageService.clearObservations() execute even when cleanupDatabase() throws,
while preserving barrierControl.disarm() execution and allowing teardown
failures to be reported appropriately.
🪄 Autofix (Beta)
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: f118440e-daa9-4a98-b369-73f39469f762
📒 Files selected for processing (5)
build.gradledocs/test-results/pr-2.2-minio-concurrency-integration-test.mdsrc/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.javasrc/test/java/com/opensource/docgrid/domain/document/service/DocumentVersionUploadFacadeTest.javasrc/test/resources/application-minio-integration.yml
Comment on lines
+107
to
+113
| @AfterEach | ||
| void tearDown() throws Exception { | ||
| barrierControl.disarm(); | ||
| cleanupDatabase(); | ||
| cleanupBucket(); | ||
| storageService.clearObservations(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
DB 정리 실패와 무관하게 MinIO 정리를 실행하세요.
cleanupDatabase()가 예외를 던지면 cleanupBucket()과 관측값 초기화가 생략되어 실제 Object와 bucket이 남고 다음 테스트를 오염시킵니다.
수정 예시
void tearDown() throws Exception {
barrierControl.disarm();
- cleanupDatabase();
- cleanupBucket();
- storageService.clearObservations();
+ try {
+ cleanupDatabase();
+ } finally {
+ try {
+ cleanupBucket();
+ } finally {
+ storageService.clearObservations();
+ }
+ }
}📝 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.
Suggested change
| @AfterEach | |
| void tearDown() throws Exception { | |
| barrierControl.disarm(); | |
| cleanupDatabase(); | |
| cleanupBucket(); | |
| storageService.clearObservations(); | |
| } | |
| `@AfterEach` | |
| void tearDown() throws Exception { | |
| barrierControl.disarm(); | |
| try { | |
| cleanupDatabase(); | |
| } finally { | |
| try { | |
| cleanupBucket(); | |
| } finally { | |
| storageService.clearObservations(); | |
| } | |
| } | |
| } |
🤖 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/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java`
around lines 107 - 113, Update tearDown() so cleanupBucket() and
storageService.clearObservations() execute even when cleanupDatabase() throws,
while preserving barrierControl.disarm() execution and allowing teardown
failures to be reported appropriately.
Comment on lines
+126
to
+140
| ExecutorService executor = Executors.newFixedThreadPool(CONCURRENT_REQUESTS); | ||
| DocumentUploadResponse firstResponse; | ||
| DocumentUploadResponse secondResponse; | ||
| try { | ||
| Future<DocumentUploadResponse> first = executor.submit( | ||
| () -> uploadAndTrack(firstRequest) | ||
| ); | ||
| Future<DocumentUploadResponse> second = executor.submit( | ||
| () -> uploadAndTrack(secondRequest) | ||
| ); | ||
| firstResponse = first.get(FUTURE_TIMEOUT_SECONDS, TimeUnit.SECONDS); | ||
| secondResponse = second.get(FUTURE_TIMEOUT_SECONDS, TimeUnit.SECONDS); | ||
| } finally { | ||
| executor.shutdownNow(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
정리 전에 Executor 종료를 확실히 기다리세요.
shutdownNow()는 interrupt만 요청하므로 작업 종료를 보장하지 않습니다. timeout이나 예외 발생 시 worker가 DB·MinIO를 계속 변경하는 동안 @AfterEach가 정리를 시작할 수 있습니다. 두 경로 모두 future 취소 후 awaitTermination으로 종료를 확인하세요.
src/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java#L126-L140: 신규 문서 경합 Executor의 종료를 기다리세요.src/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java#L188-L201: 버전 경합 Executor에도 동일한 종료 절차를 적용하세요.
📍 Affects 1 file
src/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java#L126-L140(this comment)src/test/java/com/opensource/docgrid/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java#L188-L201
🤖 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/domain/document/integration/minio/MinioUploadConcurrencyIntegrationTest.java`
around lines 126 - 140, Update both ExecutorService cleanup blocks in
MinioUploadConcurrencyIntegrationTest.java (lines 126-140 and 188-201): cancel
the submitted futures during cleanup, call shutdownNow(), and awaitTermination
before allowing teardown to continue, handling interruption while preserving the
interrupt status.
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.
변경 사항
minioIntegrationTestGradle task 및 전용 profile을 추가했습니다.DocumentVersionUploadFacade의 저장 실패·DB 실패·보상 삭제 분기 단위 테스트 6개를 추가했습니다.docs/test-results/에 기록했습니다.배경
기존 통합 테스트는 실제 DB를 사용했지만 파일 저장소는 Mock이어서
delete()호출만 확인할 수 있었습니다. DB Unique Constraint는 FileObject 중복 row를 막지만, MinIO에 먼저 저장된 경합 패자 Object의 실제 삭제까지 보장하지 않습니다.이번 PR은 실제 MinIO에 후보 두 개를 저장한 뒤 DB 경합을 발생시키고, 패자 후보가 실제로 사라지며 DB의
object_key와 잔존 Object Key가 일치하는지 검증합니다.검증 결과
./gradlew test성공./gradlew minioIntegrationTest성공테스트 시나리오
신규 Document 경합
새 Version 경합
DOCUMENT_VERSION_IN_PROGRESS실패 1건current_version_id유지문서
제외 범위
프로세스 강제 종료와 기존 고아 Object Reconciliation은 후속 운영 기능으로 분리합니다.
Closes #27