Analysis async sweep/credit 안정성 개선 (#271) - #272
Merged
Conversation
|
Warning Review limit reached
Next review available in: 33 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthrough비동기 분석 흐름이 Changes비동기 분석 작업 수명 주기
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 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: 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/main/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinator.java`:
- Around line 80-103: The sweepTimedOutTaskIds loop can repeatedly load the same
failed first batch because sweepTimedOutTask leaves failed tasks eligible for
selection. Update the coordinator to use keyset pagination or track and exclude
task IDs already processed during the current sweep, ensuring progress and
termination; add a test in AnalysisAsyncTaskSweepCoordinatorTest covering all
100 IDs failing and verifying the sweep does not repeat the batch and either
terminates or advances to the next batch.
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/service/async/AnalysisAsyncTaskService.java`:
- Around line 199-209: Update reopenPublishFailureTask to make the
recoverable-state validation and reopenForRepublish transition atomic by using a
PESSIMISTIC_WRITE-locked task lookup or an equivalent conditional update. Ensure
only the request that successfully performs the transition calls
publishAfterCommit and triggers processing; concurrent retries must not
republish the same task. Add a concurrency test covering simultaneous
resubmission attempts.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c2c487e8-81a6-4c9a-af7e-81105cc149dc
📒 Files selected for processing (9)
src/main/java/com/jobdri/jobdri_api/domain/analysis/application/usecase/async/AnalysisAsyncUseCase.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/entity/AnalysisAsyncTask.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinator.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/repository/AnalysisAsyncTaskRepository.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/async/AnalysisAsyncSweepService.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/async/AnalysisAsyncTaskService.javasrc/main/java/com/jobdri/jobdri_api/global/config/ClockConfig.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinatorTest.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/async/AnalysisAsyncFacadeServiceTest.java
Comment on lines
+80
to
+103
| private int sweepTimedOutTaskIds(TaskIdBatchLoader taskIdBatchLoader) { | ||
| int expiredCount = 0; | ||
| for (AnalysisAsyncTask task : analysisAsyncTaskRepository.findByStatusIn(EnumSet.of(AnalysisAsyncTaskStatus.PENDING, AnalysisAsyncTaskStatus.RUNNING))) { | ||
| try { | ||
| expiredCount += transactionTemplate.execute(status -> sweepTimedOutTask(task.getTaskId())); | ||
| } catch (RuntimeException e) { | ||
| log.error("Analysis async task sweep failed for taskId={}", task.getTaskId(), e); | ||
| while (true) { | ||
| List<String> taskIds = taskIdBatchLoader.load(); | ||
| if (taskIds.isEmpty()) { | ||
| return expiredCount; | ||
| } | ||
| for (String taskId : taskIds) { | ||
| expiredCount += sweepTimedOutTask(taskId); | ||
| } | ||
| if (taskIds.size() < SWEEP_BATCH_SIZE) { | ||
| return expiredCount; | ||
| } | ||
| } | ||
| return expiredCount; | ||
| } | ||
|
|
||
| private int sweepTimedOutTask(String taskId) { | ||
| try { | ||
| return transactionTemplate.execute(status -> sweepTimedOutTaskInTransaction(taskId)); | ||
| } catch (RuntimeException e) { | ||
| log.error("Analysis async task sweep failed for taskId={}", taskId, e); | ||
| return 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
실패한 첫 batch가 무한 반복될 수 있습니다.
처리 실패 시 task는 PENDING 또는 RUNNING 상태로 남습니다. 다음 반복도 PageRequest.of(0, SWEEP_BATCH_SIZE)를 사용하므로 같은 첫 100개 ID를 다시 조회합니다. 이 상태가 지속되면 sweep 스레드가 종료하지 않습니다.
src/main/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinator.java#L80-L103: 실패한 ID를 한 sweep에서 다시 처리하지 않도록 keyset pagination 또는 처리 완료 ID 추적을 적용하세요.src/test/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinatorTest.java#L94-L97: 100개 ID가 모두 실패하는 경우 sweep가 같은 batch를 반복하지 않고 종료하거나 다음 batch로 진행하는 테스트를 추가하세요.
As per path instructions, 비동기 처리 안정성 및 실패 복구 검증 우선 지침을 적용했습니다.
📍 Affects 2 files
src/main/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinator.java#L80-L103(this comment)src/test/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinatorTest.java#L94-L97
🤖 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/java/com/jobdri/jobdri_api/domain/analysis/infrastructure/async/AnalysisAsyncTaskSweepCoordinator.java`
around lines 80 - 103, The sweepTimedOutTaskIds loop can repeatedly load the
same failed first batch because sweepTimedOutTask leaves failed tasks eligible
for selection. Update the coordinator to use keyset pagination or track and
exclude task IDs already processed during the current sweep, ensuring progress
and termination; add a test in AnalysisAsyncTaskSweepCoordinatorTest covering
all 100 IDs failing and verifying the sweep does not repeat the batch and either
terminates or advances to the next batch.
Source: Path instructions
shinae1023
changed the title
9 tasks
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.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
analysis async sweep와 credit 상태 전이 쪽에서 남아 있던 구조적 결합과 멱등성 취약 지점을 정리한 PR입니다.
기존에는 sweep가 전체
PENDING/RUNNINGtask 엔티티를 읽고 있었고, credit refund 책임이 여러 클래스에 흩어져 있었으며, release 후 재예약 시 같은 reference를 재사용할 수 있어 크레딧 차감/환불 흐름이 불안정할 여지가 있었습니다.이번 PR에서는 아래 4개 커밋을 포함합니다.
[Refactor] async timeout 조회를 batch query로 분리 (#271)[Refactor] async sweep 시간 의존성을 Clock으로 치환 (#271)[Refactor] async credit refund 책임 공용화 (#271)[Fix] async credit 상태 전이 멱등성 보강 (#271)주요 변경 사항은 아래와 같습니다.
taskId만 조회하는 pending/running 전용 batch query 추가Clockbean 및 coordinator 주입 추가로 sweep 기준 시간 테스트 가능화AnalysisAsyncCreditCoordinator를 도입해 refund 책임을 공용 협력자로 통합credit_reference_version필드 및 versioned async reference 규칙 추가credit_reference_idunique index 추가📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [ #271 ]