[Feat] worker 결과 durable storage 및 멱등 완료 흐름 추가 (#133)#134
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds persistent worker task results with generated/delivered states, retry metadata, JSON payload storage, and retrieval APIs. Analysis and job posting worker bridges now store results, validate task metadata, track delivery outcomes, and expose internal result endpoints. ChangesWorker task result lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Worker
participant InternalController
participant WorkerBridgeService
participant WorkerTaskResultService
participant Database
Worker->>InternalController: POST task result
InternalController->>WorkerBridgeService: validate and store result
WorkerBridgeService->>WorkerTaskResultService: upsertGenerated
WorkerTaskResultService->>Database: serialize and save result
Worker->>InternalController: GET task result
InternalController->>WorkerBridgeService: retrieve stored result
WorkerBridgeService->>WorkerTaskResultService: get(taskId)
WorkerTaskResultService->>Database: load result
Database-->>InternalController: WorkerTaskResultResponse
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeService.java (1)
114-140: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftReorder validation and address transaction rollback on failure.
There are two critical issues in this flow:
- Mutation before validation:
workerTaskResultService.upsertGenerated(lines 114-118) is called before verifying if theuserIdandmockApplyIdmatch the task (lines 120-125). While the subsequent exception currently rolls back the transaction (preventing unauthorized writes), relying on transaction rollback for authorization is fragile. The validation block must be moved before the mutation.- Transaction rollback discards intended state updates: If
task.getStatus() == TaskStatus.FAILED, the code explicitly callsmarkDeliveryFailedIfPresentand then immediately throws aGeneralException. Because this method is annotated with@Transactional, throwing a runtime exception rolls back the entire transaction. As a result, both theupsertGeneratedandmarkDeliveryFailedIfPresentdatabase updates are discarded, defeating the purpose of persisting the failed delivery attempt.To preserve these state changes while returning an error, you must ensure the result storage operations commit independently (e.g., executing them in a separate service method annotated with
@Transactional(propagation = Propagation.REQUIRES_NEW)) before throwing the exception, or return a designated error response object instead.🐛 Proposed fix for the validation order (Issue `#1`)
`@Transactional` public AnalysisResponse completeTask(String taskId, AnalysisWorkerCompleteRequest request) { + AnalysisAsyncTask task = getTask(taskId); + if (!task.getUserId().equals(request.userId()) || !task.getMockApplyId().equals(request.mockApplyId())) { + throw new GeneralException( + GeneralErrorCode.FORBIDDEN, + "자소서 분석 worker 완료 요청 정보가 작업 정보와 일치하지 않습니다." + ); + } + workerTaskResultService.upsertGenerated( TaskType.ANALYSIS_COMPLETE, taskId, new AnalysisWorkerResultStoreRequest(request.userId(), request.mockApplyId(), request.llmResponse()) ); - AnalysisAsyncTask task = getTask(taskId); - if (!task.getUserId().equals(request.userId()) || !task.getMockApplyId().equals(request.mockApplyId())) { - throw new GeneralException( - GeneralErrorCode.FORBIDDEN, - "자소서 분석 worker 완료 요청 정보가 작업 정보와 일치하지 않습니다." - ); - } if (task.getStatus() == TaskStatus.SUCCEEDED) {🤖 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/service/AnalysisWorkerBridgeService.java` around lines 114 - 140, Move the task identity validation in the worker completion flow before calling workerTaskResultService.upsertGenerated, using the existing getTask lookup and userId/mockApplyId checks. Ensure result-storage updates in the FAILED branch, including markDeliveryFailedIfPresent and any required upsertGenerated persistence, execute in an independent REQUIRES_NEW transaction that commits before the GeneralException is thrown; update the relevant workerTaskResultService method or service boundary without changing the existing success behavior.
🧹 Nitpick comments (2)
src/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeServiceTest.java (1)
188-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider verifying
upsertGeneratedas well.While this test correctly verifies that
markDeliveredIfPresentis called, it would be more robust to also assert that the payload was saved viaupsertGeneratedprior to marking the delivery state.♻️ Proposed test augmentation
analysisWorkerBridgeService.completeTask(task.getTaskId(), request); + verify(workerTaskResultService).upsertGenerated( + eq(TaskType.ANALYSIS_COMPLETE), + eq(task.getTaskId()), + any(AnalysisWorkerResultStoreRequest.class) + ); verify(workerTaskResultService).markDeliveredIfPresent(TaskType.ANALYSIS_COMPLETE, task.getTaskId()); }🤖 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/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeServiceTest.java` around lines 188 - 211, Enhance completeTaskMarksDeliveredForSucceededTask to also verify that workerTaskResultService.upsertGenerated is invoked with the expected analysis result payload before markDeliveredIfPresent. Keep the existing delivery-state verification and use the actual generated-result arguments produced by completeTask.ops/db/migrations/20260719_worker_task_results.sql (1)
5-5: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winVerify maximum storage capacity for
result_payload.The
result_payloadcolumn is defined asTEXT. If your database engine is MySQL or MariaDB, theTEXTdata type is strictly limited to 64 KB (65,535 bytes). Given that the payload stores JSON containing potentially large LLM responses (AnalysisLlmResponse), the payload size could easily exceed 64 KB, leading to data truncation or insertion errors.If using MySQL/MariaDB, consider changing this to
MEDIUMTEXT(16 MB) orLONGTEXTto safely accommodate large LLM outputs. (If you are using PostgreSQL,TEXTsupports up to 1 GB, and this is fine.)🤖 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 `@ops/db/migrations/20260719_worker_task_results.sql` at line 5, Verify the database engine used by the migration before finalizing the result_payload column type. If it targets MySQL or MariaDB, update result_payload from TEXT to MEDIUMTEXT or LONGTEXT to accommodate large AnalysisLlmResponse JSON payloads; retain TEXT only when the target engine is PostgreSQL.
🤖 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/workerresult/entity/WorkerTaskResult.java`:
- Around line 35-37: Remove the `@Lob` annotation from the resultPayload field in
WorkerTaskResult and retain its non-nullable `@Column` mapping. Use a plain String
mapping so Hibernate maps result_payload as the existing PostgreSQL TEXT column
rather than LOB/oid semantics.
In
`@src/main/java/com/jobdri/jobdri_api/domain/workerresult/service/WorkerTaskResultService.java`:
- Around line 22-42: Update the transactional annotations on the write methods
upsertGenerated and markDeliveryFailedIfPresent to use Propagation.REQUIRES_NEW,
ensuring their persistence commits independently of any caller transaction.
Leave markDeliveredIfPresent with its existing propagation behavior.
---
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeService.java`:
- Around line 114-140: Move the task identity validation in the worker
completion flow before calling workerTaskResultService.upsertGenerated, using
the existing getTask lookup and userId/mockApplyId checks. Ensure result-storage
updates in the FAILED branch, including markDeliveryFailedIfPresent and any
required upsertGenerated persistence, execute in an independent REQUIRES_NEW
transaction that commits before the GeneralException is thrown; update the
relevant workerTaskResultService method or service boundary without changing the
existing success behavior.
---
Nitpick comments:
In `@ops/db/migrations/20260719_worker_task_results.sql`:
- Line 5: Verify the database engine used by the migration before finalizing the
result_payload column type. If it targets MySQL or MariaDB, update
result_payload from TEXT to MEDIUMTEXT or LONGTEXT to accommodate large
AnalysisLlmResponse JSON payloads; retain TEXT only when the target engine is
PostgreSQL.
In
`@src/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeServiceTest.java`:
- Around line 188-211: Enhance completeTaskMarksDeliveredForSucceededTask to
also verify that workerTaskResultService.upsertGenerated is invoked with the
expected analysis result payload before markDeliveredIfPresent. Keep the
existing delivery-state verification and use the actual generated-result
arguments produced by completeTask.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a306296c-2cd7-4b8d-979a-bfdefb7f70f7
📒 Files selected for processing (14)
ops/db/migrations/20260719_worker_task_results.sqlsrc/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisWorkerInternalController.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/dto/worker/AnalysisWorkerResultStoreRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/controller/JobPostingWorkerInternalController.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/worker/JobPostingWorkerResultStoreRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingWorkerBridgeService.javasrc/main/java/com/jobdri/jobdri_api/domain/workerresult/dto/WorkerTaskResultResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/workerresult/entity/WorkerTaskResult.javasrc/main/java/com/jobdri/jobdri_api/domain/workerresult/repository/WorkerTaskResultRepository.javasrc/main/java/com/jobdri/jobdri_api/domain/workerresult/service/WorkerTaskResultService.javasrc/main/java/com/jobdri/jobdri_api/global/apiPayload/code/GeneralErrorCode.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisWorkerBridgeServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingWorkerBridgeServiceTest.java
1 participant
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
변경 내용
기대 효과
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#133 ]
Summary by CodeRabbit
New Features
Bug Fixes
Tests