Conversation
…o OT-335-feature/transcoding-outbox-pattern
There was a problem hiding this comment.
Pull request overview
영상 업로드(트랜스코딩 요청) 발행 유실을 줄이기 위해 Outbox 패턴을 도입하고, 다중 서버 환경에서 스케줄러 중복 실행을 방지하기 위해 ShedLock을 추가하는 PR입니다.
Changes:
- 트랜스코딩 요청을
transcode_outbox테이블에 적재하고, 폴러가 주기적으로 발행하도록 변경 OutboxPoller에 ShedLock 적용 및 shedlock 테이블/설정 추가- Contents 업로드 완료 플로우에서 “즉시 MQ 발행”을 제거하고 “outbox 저장”으로 전환
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/infra-db/src/main/resources/db/migration/V12__transcode_outbox.sql | transcode_outbox/shedlock 테이블 및 인덱스/FK 추가 |
| modules/domain/src/main/java/com/ott/domain/outbox/repository/TranscodeOutboxRepository.java | PENDING outbox 배치 조회용 Spring Data 메서드 추가 |
| modules/domain/src/main/java/com/ott/domain/outbox/domain/TranscodeOutbox.java | Outbox 엔티티/상태 전이 로직 추가 |
| modules/domain/src/main/java/com/ott/domain/outbox/domain/OutboxStatus.java | Outbox 상태 enum 추가 |
| apps/api-admin/src/main/java/com/ott/api_admin/outbox/writer/OutboxWriter.java | outbox 상태 업데이트 헬퍼 추가 |
| apps/api-admin/src/main/java/com/ott/api_admin/outbox/poller/OutboxPoller.java | 주기 폴링 + MQ 발행 + ShedLock 적용 |
| apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsWriter.java | ingest job 생성 시 outbox 적재 추가 |
| apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java | 업로드 완료 시 즉시 발행 제거, outbox 저장으로 전환 |
| apps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.java | ShedLock LockProvider 설정 추가 |
| apps/api-admin/build.gradle | ShedLock 의존성 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| log.warn("Outbox 발행 실패 - outboxId: {}, retryCount: {}/{}, error: {}", | ||
| transcodeOutbox.getId(), transcodeOutbox.getRetryCount(), transcodeOutbox.getMaxRetries(), e.getMessage()); |
Walkthrough아웃박스 패턴을 도입해 업로드 시 트랜스코딩 메시지 발행을 즉시 수행하던 흐름을 변경했습니다. TranscodeOutbox 엔티티·리포지토리·마이그레이션을 추가하고, OutboxPoller와 OutboxWriter를 통해 주기적 폴링 및 발행을 수행하도록 변경했습니다. ShedLock과 스케줄링이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as Content/ShortForm<br/>Service
participant Writer as Content/ShortForm<br/>Writer
participant OutboxRepo as TranscodeOutbox<br/>Repository
participant Poller as OutboxPoller<br/>(ShedLock)
participant RabbitPublisher as RabbitTranscode<br/>Publisher
participant OutboxWriter as OutboxWriter
Client->>Service: completeOriginUpload()
Service->>Writer: createIngestJobWithOutbox()
Writer->>Writer: create IngestJob
Writer->>OutboxRepo: save TranscodeOutbox (PENDING)
OutboxRepo-->>Writer: saved
Writer-->>Service: IngestJobResult
Service-->>Client: Success
loop Every 10s (ShedLock)
Poller->>OutboxRepo: findTop50ByOutboxStatusOrderByCreatedDateAsc(PENDING)
OutboxRepo-->>Poller: List<TranscodeOutbox>
Poller->>Poller: toMessage()
Poller->>RabbitPublisher: publish(TranscodeMessage)
alt publish success
RabbitPublisher-->>Poller: ack
Poller->>OutboxWriter: markAsPublished(id)
OutboxWriter->>OutboxRepo: load & update status=PUBLISHED
else publish failure
RabbitPublisher-->>Poller: error
Poller->>OutboxWriter: markAsFailed(id, error)
OutboxWriter->>OutboxRepo: load & update retryCount, maybe status=FAILED
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java (1)
37-37:⚠️ Potential issue | 🟡 Minor
RabbitTranscodePublisher주입이 사용되지 않으므로 제거하세요.Outbox 패턴으로 마이그레이션되면서 이 필드(37번 줄)는 더 이상 사용되지 않습니다.
completeShortFormOriginUpload메서드에서writer.createIngestJobWithOutbox()만 호출되며, 직접 메시지 발행 로직은 제거되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` at line 37, BackOfficeShortFormService has an unused RabbitTranscodePublisher field; remove the private final RabbitTranscodePublisher transcodePublisher declaration and any constructor parameter, assignment, or import related to RabbitTranscodePublisher, leaving the service to call writer.createIngestJobWithOutbox() in completeShortFormOriginUpload() only; ensure no remaining references (field, constructor arg, or import) remain and update any tests or usages that expect the publisher to be injected.apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormWriter.java (1)
207-226:⚠️ Potential issue | 🟠 Major중복 완료 요청에서 ingestJob/outbox가 이중 생성될 수 있습니다.
existsByMediaId()선조회는 원자적이지 않아서, 같은shortFormId완료 요청이 동시에 들어오면 두 트랜잭션이 둘 다false를 보고 각각IngestJob과TranscodeOutbox를 만들 수 있습니다. ShedLock는 poller 중복만 막아주므로 여기에는 효력이 없습니다.ingest_job.media_id같은 DB unique constraint로 막고 중복 키를ALREADY_INGESTED로 변환하는 쪽이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormWriter.java` around lines 207 - 226, The pre-check using ingestJobRepository.existsByMediaId(media.getId()) is racy and can allow duplicate IngestJob/TranscodeOutbox creation; add a DB-level unique constraint on ingest_job.media_id and change the save path (IngestJob.builder... ingestJobRepository.save and subsequent transcodeOutboxRepository.save) to run in a single transaction and catch the database unique-key exception (e.g., DataIntegrityViolationException / DuplicateKeyException) thrown by the IngestJob save, converting it into a BusinessException(ErrorCode.ALREADY_INGESTED); ensure the same defensive handling applies if the outbox insert can violate a unique constraint so duplicates are mapped to ALREADY_INGESTED rather than creating duplicate records.
🧹 Nitpick comments (2)
apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsWriter.java (1)
159-176: Outbox 생성 로직이 올바르게 구현되었습니다.IngestJob과 Outbox가 동일 트랜잭션 내에서 저장되어 원자성이 보장됩니다. 로깅에 모든 관련 ID가 포함되어 추적성이 좋습니다.
fileSize계산이 두 번 수행됩니다(Line 164, 174). 로컬 변수로 추출하면 중복을 줄일 수 있습니다.♻️ fileSize 계산 중복 제거 제안
+ long fileSizeBytes = (long) contents.getVideoSize() * 1024; + // 2. Outbox 저장 TranscodeOutbox outbox = TranscodeOutbox.builder() .mediaId(media.getId()) .ingestJobId(ingestJob.getId()) .originUrl(originObjectKey) - .fileSize((long) contents.getVideoSize() * 1024) + .fileSize(fileSizeBytes) .mediaType(MediaType.CONTENTS) .build(); transcodeOutboxRepository.save(outbox); log.info("IngestJob + Outbox 생성 - contentsId: {}, mediaId: {}, ingestJobId: {}, outboxId: {}", contentsId, media.getId(), ingestJob.getId(), outbox.getId()); return new IngestJobResult( media.getId(), ingestJob.getId(), - originObjectKey, (long) contents.getVideoSize() * 1024, + originObjectKey, fileSizeBytes, MediaType.CONTENTS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsWriter.java` around lines 159 - 176, Compute the fileSize once into a local long variable (e.g., long fileSize = (long) contents.getVideoSize() * 1024) and use that variable in both the TranscodeOutbox.builder().fileSize(...) call and the IngestJobResult(...) return so the duplicated expression using contents.getVideoSize() is removed; update references in TranscodeOutbox, the log call if needed, and the IngestJobResult to use the new fileSize variable.apps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.java (1)
10-10: 사용되지 않는 import 제거 필요.
javax.sql.DataSourceimport가 사용되지 않습니다.🧹 사용되지 않는 import 제거
-import javax.sql.DataSource;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.java` at line 10, Remove the unused import javax.sql.DataSource from the ShedLockConfig class; locate the import statement near the top of ShedLockConfig (the line importing javax.sql.DataSource) and delete it or run your IDE's "organize imports" to eliminate the unused import without changing any other code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-admin/build.gradle`:
- Around line 25-27: The ShedLock dependencies currently pinned to
'net.javacrumbs.shedlock:shedlock-spring:5.10.0' and
'net.javacrumbs.shedlock:shedlock-provider-jdbc-template:5.10.0' are outdated;
update those dependency coordinates to the current stable release (e.g., 7.6.0)
in the build file, ensure both artifacts use the same new version, then run the
project's build and integration tests to resolve any API or configuration
changes introduced by the upgrade and adjust ShedLock configuration/code where
compilation or behavioral changes surface (look for usages of ShedLock
annotations/config classes to update).
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java`:
- Around line 72-75: The current flow calls
uploadHelper.completeMultipartUpload(...) then
writer.createIngestJobWithOutbox(...), so if createIngestJobWithOutbox throws
the S3 multipart is complete but DB/outbox is missing; make the operation
idempotent or add a recovery path: after completeMultipartUpload(objectKey, ...)
ensure createIngestJobWithOutbox(contentsId, objectKey) is safe to retry by
first checking for an existing IngestJob/outbox record (lookup by objectKey or
contentsId) and returning existing IngestJobResult if present, or implement a
small durable “pending-outbox” record written before/after S3 completion that
can be used by a separate replay endpoint to call createIngestJobWithOutbox
safely; update createIngestJobWithOutbox (and any unique constraints on the
IngestJob/outbox table) to be idempotent so repeated calls do not create
duplicates and will return the created/existing IngestJobResult.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/outbox/poller/OutboxPoller.java`:
- Around line 60-61: The log prints the pre-update retry count because
transcodeOutbox.getRetryCount() is called before markAsFailed() updates it; call
markAsFailed() first and then log the updated value (either by re-reading
transcodeOutbox.getRetryCount() after markAsFailed() or by using the return
value from markAsFailed() if it returns the updated entity), ensuring the
log.warn that currently references transcodeOutbox.getId(),
transcodeOutbox.getRetryCount(), transcodeOutbox.getMaxRetries(), and
e.getMessage() uses the post-markAsFailed() retryCount.
- Around line 49-62: The outbox publish flow in OutboxPoller is not atomic:
rabbitTranscodePublisher.publish(...) may succeed while
outboxWriter.markAsPublished(...) fails, causing duplicate delivery; change the
flow so state update and publish are performed atomically — e.g., add a new
transactional operation in OutboxPoller that (1) obtains a DB lock or starts a
transaction for the TranscodeOutbox record, (2) marks the record as
“publishing”/reserved or updates a published flag in the same transaction, then
(3) calls rabbitTranscodePublisher.publish(...) and on successful publish
commits the transaction (or sets published=true), and on publish failure rolls
back or marks failed via outboxWriter.markAsFailed(...). Use/extend methods like
outboxWriter.markAsPublished, outboxWriter.markAsFailed, or introduce a new
outboxWriter.reserveForPublish/markPublishedTransactional helper and ensure
rabbitTranscodePublisher.publish is used with publisher confirms so publish
success is known before committing.
In
`@modules/domain/src/main/java/com/ott/domain/outbox/repository/TranscodeOutboxRepository.java`:
- Around line 13-14: The current repository query
findTop50ByOutboxStatusOrderByCreatedDateAsc(status) will keep returning old
failed items that remain PENDING (as set by TranscodeOutbox.markFailed),
blocking newer events; change the selection to only return items whose
nextAttemptAt is <= now (or null) so retries respect backoff, e.g. add a
condition on nextAttemptAt (or split into two queries: new PENDING items and
retryable PENDING items with nextAttemptAt <= now) and update the repository
method(s) accordingly so polling does not fetch records that are not yet
eligible for retry.
---
Outside diff comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java`:
- Line 37: BackOfficeShortFormService has an unused RabbitTranscodePublisher
field; remove the private final RabbitTranscodePublisher transcodePublisher
declaration and any constructor parameter, assignment, or import related to
RabbitTranscodePublisher, leaving the service to call
writer.createIngestJobWithOutbox() in completeShortFormOriginUpload() only;
ensure no remaining references (field, constructor arg, or import) remain and
update any tests or usages that expect the publisher to be injected.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormWriter.java`:
- Around line 207-226: The pre-check using
ingestJobRepository.existsByMediaId(media.getId()) is racy and can allow
duplicate IngestJob/TranscodeOutbox creation; add a DB-level unique constraint
on ingest_job.media_id and change the save path (IngestJob.builder...
ingestJobRepository.save and subsequent transcodeOutboxRepository.save) to run
in a single transaction and catch the database unique-key exception (e.g.,
DataIntegrityViolationException / DuplicateKeyException) thrown by the IngestJob
save, converting it into a BusinessException(ErrorCode.ALREADY_INGESTED); ensure
the same defensive handling applies if the outbox insert can violate a unique
constraint so duplicates are mapped to ALREADY_INGESTED rather than creating
duplicate records.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.java`:
- Line 10: Remove the unused import javax.sql.DataSource from the ShedLockConfig
class; locate the import statement near the top of ShedLockConfig (the line
importing javax.sql.DataSource) and delete it or run your IDE's "organize
imports" to eliminate the unused import without changing any other code.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsWriter.java`:
- Around line 159-176: Compute the fileSize once into a local long variable
(e.g., long fileSize = (long) contents.getVideoSize() * 1024) and use that
variable in both the TranscodeOutbox.builder().fileSize(...) call and the
IngestJobResult(...) return so the duplicated expression using
contents.getVideoSize() is removed; update references in TranscodeOutbox, the
log call if needed, and the IngestJobResult to use the new fileSize variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2e576be8-1915-4816-9903-fef688135d4c
📒 Files selected for processing (12)
apps/api-admin/build.gradleapps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsWriter.javaapps/api-admin/src/main/java/com/ott/api_admin/outbox/poller/OutboxPoller.javaapps/api-admin/src/main/java/com/ott/api_admin/outbox/writer/OutboxWriter.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormWriter.javamodules/domain/src/main/java/com/ott/domain/outbox/domain/OutboxStatus.javamodules/domain/src/main/java/com/ott/domain/outbox/domain/TranscodeOutbox.javamodules/domain/src/main/java/com/ott/domain/outbox/repository/TranscodeOutboxRepository.javamodules/infra-db/src/main/resources/db/migration/V12__transcode_outbox.sql
| import javax.sql.DataSource; | ||
|
|
||
| @Configuration | ||
| @EnableSchedulerLock(defaultLockAtMostFor = "10m") |
There was a problem hiding this comment.
@EnableScheduling
이거 안 달아도 작동하나요?
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java (1)
34-34: 사용되지 않는transcodePublisher필드를 제거하세요.아웃박스 패턴 도입으로 직접 publish 호출이 제거되었지만,
RabbitTranscodePublisher transcodePublisher필드는 여전히 주입되고 있습니다. 사용되지 않는 의존성이므로 제거해야 합니다.♻️ 제안된 수정
private final BackOfficeContentsReader reader; private final BackOfficeContentsWriter writer; private final UploadHelper uploadHelper; -private final RabbitTranscodePublisher transcodePublisher;또한 import 문도 제거:
-import com.ott.api_admin.publish.RabbitTranscodePublisher;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java` at line 34, Remove the unused RabbitTranscodePublisher dependency: in BackOfficeContentsService delete the private final RabbitTranscodePublisher transcodePublisher field and its constructor/injection parameter (if present), update any constructors or `@Autowired` usages accordingly, and remove the unused import for RabbitTranscodePublisher so there are no dangling references or unused imports left.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java`:
- Line 34: Remove the unused RabbitTranscodePublisher dependency: in
BackOfficeContentsService delete the private final RabbitTranscodePublisher
transcodePublisher field and its constructor/injection parameter (if present),
update any constructors or `@Autowired` usages accordingly, and remove the unused
import for RabbitTranscodePublisher so there are no dangling references or
unused imports left.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 33e4f07f-9efd-43f3-858f-78b8c4039a01
📒 Files selected for processing (4)
apps/api-admin/src/main/java/com/ott/api_admin/ApiAdminApplication.javaapps/api-admin/src/main/java/com/ott/api_admin/config/shedlock/ShedLockConfig.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/outbox/poller/OutboxPoller.java
📝 작업 내용
📷 스크린샷
☑️ 체크 리스트
#️⃣ 연관된 이슈
closes #183
💬 리뷰 요구사항
1. 발행 쪽 유실 방지를 위한 아웃박스 패턴 도입
2. 다중 서버 환경 스케줄러 중복 실행 방지
Summary by CodeRabbit
새로운 기능
개선사항