Merged
Conversation
…o OT-163-feature/transcode-improvement
…o OT-163-feature/transcode-improvement
…o OT-163-feature/transcode-improvement
Walkthrough트랜스코딩 서버를 command 기반 오케스트레이션으로 마이그레이션합니다. 기존의 직접 파이프라인 호출을 Command 추출 및 실행 메커니즘으로 대체하고, 세분화된 예외 처리 및 RabbitMQ DLQ 및 재시도 정책을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant RabbitListener as RabbitTranscodeListener
participant Orchestrator as JobOrchestrator
participant Validator as FileValidator/<br/>StreamValidator
participant Extractor as CommandExtractor
participant Factory as CommandPipelineFactory
participant Pipeline as CommandPipeline<br/>(TranscodeCommand)
participant FFmpeg as FFmpegExecutor
RabbitListener->>Orchestrator: handle(TranscodeMessage)
Orchestrator->>Validator: validateDiskSpace()
Validator-->>Orchestrator: ✓ or throw FatalException
Orchestrator->>Orchestrator: createWorkDir()
Orchestrator->>Orchestrator: downloadInput()
Orchestrator->>Validator: inspectMedia(ProbeResult)
Validator-->>Orchestrator: ✓ or throw UnsupportedMediaException
Orchestrator->>Extractor: extractCommand(TranscodeMessage, ProbeResult)
Extractor-->>Orchestrator: List<Command>
loop for each Command
Orchestrator->>Factory: find(command)
Factory-->>Orchestrator: CommandPipeline<?>
Orchestrator->>Pipeline: execute(command)
Pipeline->>FFmpeg: execute(TranscodeCommand)
FFmpeg-->>Pipeline: ✓ or throw FfmpegException
Pipeline-->>Orchestrator: ✓ or throw
end
Orchestrator-->>RabbitListener: ✓ or StorageException
Note over Orchestrator: finally: cleanupWorkDir()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 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 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/transcoder/src/main/java/com/ott/transcoder/inspection/DiskSpaceGuard.java (1)
20-22:⚠️ Potential issue | 🟠 Major
check메서드가 비어있어 디스크 공간 검증이 수행되지 않습니다.Javadoc에 명시된 계약(원본 크기 × multiplier 공간 확인)과 달리
check메서드가 아무 작업도 수행하지 않습니다. 삭제된 import들(Files,FileStore,IOException)이 실제 구현에 필요했던 것으로 보입니다.이 컴포넌트가
@Component로 등록되어 주입되므로, 빈 메서드는 디스크 공간 부족 상황을 감지하지 못해 트랜스코딩 실패로 이어질 수 있습니다.디스크 공간 검증 로직 구현을 도와드릴까요? 혹은 의도적인 placeholder라면 TODO 주석이나
UnsupportedOperationException을 추가하여 명시적으로 표시하는 것이 좋겠습니다.🛠️ 기본 구현 예시
+import java.io.IOException; +import java.nio.file.FileStore; +import java.nio.file.Files; + /** * 다운로드 전 디스크 여유 공간 검증 * 원본 크기 × multiplier만큼의 공간이 있는지 확인 */ `@Slf4j` `@Component` public class DiskSpaceGuard { `@Value`("${transcoder.validation.disk-space-multiplier:5}") private double multiplier; - public void check(Path originPath) { - + public void check(Path originPath) throws IOException { + long fileSize = Files.size(originPath); + long requiredSpace = (long) (fileSize * multiplier); + FileStore store = Files.getFileStore(originPath); + long usableSpace = store.getUsableSpace(); + + if (usableSpace < requiredSpace) { + log.error("디스크 공간 부족: 필요={}, 가용={}", requiredSpace, usableSpace); + throw new IllegalStateException("디스크 공간이 부족합니다"); + } + log.debug("디스크 공간 확인 완료: 필요={}, 가용={}", requiredSpace, usableSpace); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/inspection/DiskSpaceGuard.java` around lines 20 - 22, The check method in DiskSpaceGuard is currently empty; implement it to verify available disk space by (1) using Files.size(originPath) to get the original file size, (2) multiplying by the existing multiplier field to compute required bytes, (3) obtaining the FileStore for originPath via Files.getFileStore(originPath) and calling getUsableSpace() to get available bytes, and (4) if usableSpace < requiredBytes throw an explicit runtime exception (e.g., IllegalStateException with a clear message) or a custom InsufficientDiskSpaceException; handle IOExceptions from Files methods by wrapping them in the same runtime exception so callers fail fast. Ensure imports for java.nio.file.Files, java.nio.file.FileStore and java.io.IOException are present; if this was intended as a placeholder instead, replace the empty body with a TODO comment and throw UnsupportedOperationException from DiskSpaceGuard.check to make the placeholder explicit.apps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.java (2)
75-106:⚠️ Potential issue | 🟠 Majornull 가능성을 주석으로만 남겨두면 여기서 바로 NPE가 납니다.
Line 96 주석처럼
format이 없을 수 있는데 바로format.has(...)를 호출하고 있습니다. 이 경우ProbeException/TranscodeErrorCode매핑을 우회하므로, 응답 구조를 검증한 뒤 같은 예외 계층으로 변환해 주세요.제안 코드
private ProbeResult parseJson(String json) throws IOException { JsonNode root = objectMapper.readTree(json); - JsonNode streamList = root.get("streams"); + JsonNode streamList = root.path("streams"); + if (!streamList.isArray()) { + throw new ProbeException(TranscodeErrorCode.PROBE_FAILED, + "ffprobe 응답에 streams가 없음"); + } @@ - JsonNode format = root.get("format"); // null 가능성 존재 + JsonNode format = root.path("format"); - double duration = format.has("duration") - ? format.get("duration").asDouble() - : 0.0; + double duration = format.path("duration").asDouble(0.0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.java` around lines 75 - 106, parseJson currently assumes format is non-null and calls format.has(...) which can NPE; add an explicit null check for the JsonNode format at the start of the metadata handling in parseJson and, if null, throw a ProbeException mapped to the same TranscodeErrorCode family used elsewhere (e.g., throw new ProbeException(TranscodeErrorCode.<appropriate_code>, "format metadata missing" )) so all parsing failures use the same exception mapping; after that safe-check you can call format.has(...)/format.get(...) as before and compute duration/fps/bitrate.
49-71:⚠️ Potential issue | 🔴 Critical타임아웃이 설계상 동작하지 않습니다. 스트림 버퍼 교착 상태 위험이 있습니다.
readAllBytes()는 EOF(자식 프로세스가 stdout을 닫을 때까지)에 도달할 때까지 무한정 블로킹됩니다. 자식 프로세스가 출력을 계속 생성하면 OS 파이프 버퍼가 가득 차서 프로세스가 블로킹되고, stdout도 닫지 않으므로readAllBytes()도 반환되지 않습니다. 결과적으로waitFor(2, TimeUnit.MINUTES)에 도달할 수 없어 타임아웃이 무시됩니다.필요한 수정:
- stdout/stderr를
waitFor()전에 병렬로 소비하거나,redirectErrorStream(true)로 병합하세요.- 또는
process.getOutputStream()을 먼저 닫고, 짧은 시간 제한으로 읽으세요.InterruptedException캐치 후Thread.currentThread().interrupt()로 인터럽트 플래그를 복원하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.java` around lines 49 - 71, The current implementation in ProcessBuilderFfprobeExecutor uses process.getInputStream().readAllBytes() which can block and prevent waitFor(2, TimeUnit.MINUTES) from ever seeing the process exit; change the execution to consume the child process streams asynchronously before waiting: close process.getOutputStream() immediately, start background readers (or use an ExecutorService) to drain process.getInputStream() (and process.getErrorStream() if you stop using redirectErrorStream(true)) into a String/ByteArray without blocking the main thread, then call waitFor with the timeout and handle timeout by destroyForcibly(); also in the catch for InterruptedException restore the thread interrupt flag via Thread.currentThread().interrupt() and rethrow the ProbeException so transient interrupts are preserved.
🧹 Nitpick comments (10)
apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/FfmpegExecutor.java (1)
15-23: 인터페이스 변경이 구현체와 일관성 있게 적용되었습니다.
throws IOException, InterruptedException제거는ProcessBuilderFfmpegExecutor의 예외 처리 전략과 일치합니다. 구현체에서 checked exception을FfmpegException으로 래핑하므로 인터페이스에서 throws 절을 제거하는 것이 적절합니다.선택적으로, Javadoc에
@throws FfmpegException문서를 추가하면 호출자에게 발생 가능한 런타임 예외를 명시적으로 알릴 수 있습니다.,
📝 선택적: Javadoc에 예외 문서 추가
/** * 단일 프로파일에 대해 HLS 트랜스코딩을 수행 * * `@param` inputFile 원본 영상 파일 경로 * `@param` outputDir 출력 디렉토리 (하위에 360p/, 720p/, 1080p/ 폴더가 생성됨) * `@param` profile 트랜스코딩 설정 (해상도, 비트레이트, 코덱 등) * `@return` 생성된 미디어 플레이리스트(media.m3u8) 경로 + * `@throws` FfmpegException FFmpeg 실행 실패, 타임아웃 등 트랜스코딩 오류 발생 시 */ Path execute(Path inputFile, Path outputDir, TranscodeProfile profile);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/FfmpegExecutor.java` around lines 15 - 23, Update the FfmpegExecutor interface Javadoc to explicitly document the runtime exception behavior: add an `@throws` FfmpegException line describing that implementations (e.g., ProcessBuilderFfmpegExecutor) wrap IO/Interrupted exceptions into FfmpegException so callers should expect a runtime FfmpegException from execute(Path inputFile, Path outputDir, TranscodeProfile profile); this keeps the interface consistent with the implementation and informs callers of the possible runtime exception.apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HLSTranscodePipeline.java (1)
1-55: 주석 처리된 구현은 새 파일로 남기지 않는 편이 낫습니다.현재는 컴파일되는 동작이 전혀 없고 TODO만 소스 트리에 남아 있어 실제 활성 파이프라인을 오해하기 쉽습니다. 재도입 전까지는 파일을 제거하거나, 최소한 명시적으로 실패하는 stub로 두는 편이 관리하기 쉽습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HLSTranscodePipeline.java` around lines 1 - 55, The file HLSTranscodePipeline currently contains a fully commented-out implementation which is confusing; either remove the file from the tree or convert it into an explicit failing stub so its presence is unambiguous. If you choose the stub approach, keep the class HLSTranscodePipeline and its public API (support, execute) but implement them to immediately fail (e.g., throw UnsupportedOperationException or return a clear negative result) with a log message indicating the pipeline is not implemented; ensure constructors/fields (transcodePlanner, ffmpegExecutor, masterPlaylistGenerator, videoStorage) are removed or replaced with no-op/explicit-failure behavior so the class compiles and cannot be mistaken for a working pipeline.apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.java (1)
14-17:support()는CommandType뿐 아니라 실제 타입도 같이 확인해 주세요.여기서 선택된 파이프라인은
CommandPipelineExecutor.invoke()에서 unchecked cast로 바로 실행됩니다. 지금처럼getType()만 보면 다른Command구현이 실수로TRANSCODE를 반환했을 때 런타임ClassCastException으로 깨질 수 있습니다.수정 예시
`@Override` public boolean support(Command command) { - return CommandType.TRANSCODE.equals(command.getType()); + return command instanceof TranscodeCommand + && CommandType.TRANSCODE.equals(command.getType()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.java` around lines 14 - 17, support() currently only checks Command.getType() against CommandType.TRANSCODE which allows other Command implementations to falsely claim TRANSCODE and cause a ClassCastException in CommandPipelineExecutor.invoke(); update TranscodeCommandPipeline.support(Command) to verify both the type and the actual runtime class (e.g., check command instanceof TranscodeCommand or command.getClass() == TranscodeCommand.class) before returning true so the executor's unchecked cast is safe.apps/transcoder/src/main/java/com/ott/transcoder/storage/LocalVideoStorage.java (1)
51-60: 업로드 파일 목록을 전부toList()로 모으지 않는 편이 좋습니다.HLS 산출물은 세그먼트 수가 많아서 여기서 전체 파일 트리를 한 번에 적재하면 업로드 시작이 늦어지고 메모리 피크도 커집니다. 스트림을 바로 순회하면 같은 동작을 더 낮은 비용으로 처리할 수 있습니다.
수정 예시
-import java.util.List; +import java.util.Iterator; import java.util.stream.Stream; ... try (Stream<Path> fileStream = Files.walk(localDir)) { Files.createDirectories(destination); - List<Path> fileList = fileStream.filter(Files::isRegularFile).toList(); - - for (Path file : fileList) { + Iterator<Path> iterator = fileStream.filter(Files::isRegularFile).iterator(); + while (iterator.hasNext()) { + Path file = iterator.next(); Path relativePath = localDir.relativize(file); Path targetFile = destination.resolve(relativePath); Files.createDirectories(targetFile.getParent()); Files.copy(file, targetFile, StandardCopyOption.REPLACE_EXISTING); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/storage/LocalVideoStorage.java` around lines 51 - 60, 현재 LocalVideoStorage의 Files.walk(localDir) 결과를 Stream#toList()로 전부 모으고 있어 메모리와 지연을 유발합니다; toList()를 제거하고 fileStream.filter(Files::isRegularFile).forEach(...)로 스트림을 바로 순회하도록 변경하세요; 각 파일에 대해 localDir.relativize(file), destination.resolve(relativePath), Files.createDirectories(targetFile.getParent()), Files.copy(...) 로 기존 복사 로직을 수행하되 람다 내부에서 발생하는 IOException은 UncheckedIOException으로 래핑하거나 별도 메서드로 빼서 처리(또는 외부에서 적절히 처리)하여 컴파일/예외 처리를 맞추세요.apps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/UnsupportedMediaException.java (1)
6-9: 원인 예외를 이어받는 생성자도 맞춰 두는 편이 좋습니다.동일 계층의
InvalidInputException/FfmpegException은cause오버로드가 있는데 여기만 빠져 있어서 unsupported media 판정 시 원인 체인이 끊깁니다. 진단 일관성을 위해(errorCode, message, cause)생성자도 추가해 주세요.제안 diff
public class UnsupportedMediaException extends FatalException { public UnsupportedMediaException(TranscodeErrorCode errorCode, String message) { super(errorCode, message); } + + public UnsupportedMediaException(TranscodeErrorCode errorCode, String message, Throwable cause) { + super(errorCode, message, cause); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/UnsupportedMediaException.java` around lines 6 - 9, Add an overloaded constructor to UnsupportedMediaException that accepts (TranscodeErrorCode errorCode, String message, Throwable cause) and forwards all three to the superclass constructor (FatalException) so the original cause is preserved; mirror the existing overload pattern used by InvalidInputException and FfmpegException to restore the exception chain and update any places that construct UnsupportedMediaException with a cause to use the new constructor.apps/transcoder/src/main/java/com/ott/transcoder/constant/IngestJobConstant.java (1)
7-10: 중첩 상수 클래스의 생성자를private으로 변경 권장상수만 포함하는 유틸리티 클래스의 생성자는 인스턴스화를 방지하기 위해
private이어야 합니다. 현재DirectoryConstant,ValidateConstant,VideoConstant,AudioConstant모두public생성자를 가지고 있습니다.♻️ 수정 제안
public static final class DirectoryConstant { - public DirectoryConstant() { - } + private DirectoryConstant() { + }동일하게
ValidateConstant,VideoConstant,AudioConstant에도 적용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/constant/IngestJobConstant.java` around lines 7 - 10, The nested constant classes (DirectoryConstant, ValidateConstant, VideoConstant, AudioConstant) should not be instantiable; change their public constructors to private no-arg constructors inside IngestJobConstant so the classes remain utility holders for constants only (e.g., replace the public DirectoryConstant() {} with private DirectoryConstant() {} and apply the same change to ValidateConstant, VideoConstant, and AudioConstant).apps/transcoder/src/main/java/com/ott/transcoder/JobOrchestrator.java (1)
52-53: 작업 디렉토리 경로 포맷 가독성 개선 제안현재 경로 포맷은
media-{mediaId}job-{ingestJobId}(예:media-123job-456)로,mediaId와job-사이에 구분자가 없어 가독성이 떨어집니다. 디버깅 시 식별이 어려울 수 있습니다.♻️ 개선 제안
- Path workDir = Path.of(tempDir, PREFIX_WORK_DIR + mediaId + SUFFIX_WORK_DIR + ingestJobId); + Path workDir = Path.of(tempDir, PREFIX_WORK_DIR + mediaId + "-" + SUFFIX_WORK_DIR + ingestJobId);또는 상수 자체를
SUFFIX_WORK_DIR = "-job-"로 수정하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/JobOrchestrator.java` around lines 52 - 53, The workDir path in JobOrchestrator currently concatenates PREFIX_WORK_DIR + mediaId + SUFFIX_WORK_DIR + ingestJobId producing strings like "media-123job-456"; update the formatting so there is a clear separator between mediaId and the job segment—either insert an explicit delimiter (e.g., PREFIX_WORK_DIR + mediaId + "-" + SUFFIX_WORK_DIR + ingestJobId) or change the SUFFIX_WORK_DIR constant to include the leading hyphen (e.g., SUFFIX_WORK_DIR = "-job-") so Path.of(tempDir, PREFIX_WORK_DIR + mediaId + SUFFIX_WORK_DIR + ingestJobId) yields "media-123-job-456" for easier debugging and readability.apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineFactory.java (1)
15-20:orElseThrow()에 의미 있는 예외 메시지 추가 권장파이프라인을 찾지 못했을 때
NoSuchElementException이 발생하지만, 어떤Command에 대한 파이프라인이 없는지 진단 정보가 없습니다. 디버깅과 운영 시 로그 추적을 위해 구체적인 예외를 던지는 것이 좋습니다.♻️ 개선 제안
public CommandPipeline<?> find(Command command) { return commandPipelineList.stream() .filter(p -> p.support(command)) .findFirst() - .orElseThrow(); + .orElseThrow(() -> new IllegalArgumentException( + "지원하는 파이프라인 없음 - commandType: " + command.getType())); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineFactory.java` around lines 15 - 20, find 메서드의 orElseThrow()가 어떤 Command에 대해 파이프라인을 찾지 못했는지 정보를 제공하지 않으므로, CommandPipelineFactory.find에서 commandPipelineList.stream(...).findFirst().orElseThrow() 부분을 orElseThrow(() -> new NoSuchElementException(...)) 형태로 바꿔 구체적인 메시지(예: "No pipeline found for Command: " + command 또는 command.getType())를 포함해 던지도록 변경하고, 메시지에 사용될 식별자(예: command 또는 command.getId())를 포함해 CommandPipeline.support를 호출하던 대상 Command을 명시하도록 수정하세요.apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/processbuilder/ProcessBuilderFfmpegExecutor.java (1)
67-74: 프로세스 출력 읽기와 대기 순서에 대한 잠재적 데드락 고려현재 구조에서는 프로세스 출력을 모두 읽은 후
waitFor()를 호출합니다. FFmpeg이 많은 로그를 출력하는 경우 출력 버퍼가 가득 차면 프로세스가 블록될 수 있습니다. 현재 HLS 트랜스코딩 시나리오에서는 문제가 없을 가능성이 높지만, 장기적으로 별도 스레드에서 출력을 소비하는 패턴을 고려할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/processbuilder/ProcessBuilderFfmpegExecutor.java` around lines 67 - 74, The current ProcessBuilderFfmpegExecutor reads process.getInputStream() fully on the calling thread before calling process.waitFor(), which can deadlock if FFmpeg floods stdout/err; change to consume both stdout and stderr concurrently by starting background gobbler threads (or use processBuilder.redirectErrorStream(true) if merging is acceptable) immediately after processBuilder.start(), e.g., create StreamGobbler-style Runnables for process.getInputStream() and process.getErrorStream() (used in ProcessBuilderFfmpegExecutor around processBuilder.start(), BufferedReader and process.getInputStream()/getErrorStream()), start them before invoking process.waitFor(), and ensure they are joined or completed after waitFor() to preserve logging and resource cleanup.modules/domain/src/main/java/com/ott/domain/video_profile/domain/Resolution.java (1)
9-18: 타입 일관성 고려:videoBitrate와audioBitrate
videoBitrate는long타입이고audioBitrate는String타입("96k")입니다. FFmpeg 명령어에 직접 사용하기 위한 의도적인 설계일 수 있으나, 일관성을 위해 둘 다 동일한 타입으로 통일하거나 별도의 포맷터를 사용하는 것을 고려할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/video_profile/domain/Resolution.java` around lines 9 - 18, The Resolution enum mixes types: videoBitrate is a long while audioBitrate is a String; update the enum to be consistent by changing audioBitrate to a numeric type (e.g., long) and provide a formatting accessor for FFmpeg (e.g., toFfmpegAudioBitrate() or getAudioBitrateFfmpeg()) that appends the "k" suffix when needed; modify the enum constants (P360, P720, P1080) to supply numeric audio bitrates and add the new accessor alongside existing videoBitrate/getVideoBitrate to keep callers working.
🤖 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/transcoder/src/main/java/com/ott/transcoder/command/CommandExtractor.java`:
- Around line 16-22: The current extractCommand method indiscriminately creates
a TranscodeCommand for every Resolution, ignoring message and probeResult;
instead, call the TranscodePlanner with the TranscodeMessage and ProbeResult to
get the planned resolutions/tasks and only construct TranscodeCommand instances
for those planner results (use Resolution values returned by the planner rather
than Resolution.values()), and branch optional/choice tasks based on flags in
the TranscodeMessage when creating each TranscodeCommand so optional tasks are
only included when message indicates them.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/command/TranscodeCommand.java`:
- Around line 7-20: Make TranscodeCommand fail fast on null resolution: make the
resolution field final and remove the generated all-args constructor, then add
an explicit constructor that calls Objects.requireNonNull(resolution,
"resolution must not be null") (or throws IllegalArgumentException) so new
TranscodeCommand(null) cannot be created; this ensures methods like
getCommandKey() (which calls resolution.getKey()) cannot throw a delayed NPE.
In `@apps/transcoder/src/main/java/com/ott/transcoder/config/RabbitConfig.java`:
- Around line 95-103: The comment about "3회 재시도" is misleading because
SimpleRetryPolicy(3, ...) sets maxAttempts (initial attempt + 2 retries = 3
attempts total); either update the comment to reflect "총 3회(1시도+2재시도)" or change
SimpleRetryPolicy's first argument to 4 to get 3 retries (total 4 attempts);
also remove the unused commented-out factory.setErrorHandler(new
ConditionalRejectingErrorHandler(new TranscodeFatalExceptionStrategy())) line if
it's not needed to keep the file clean, referencing SimpleRetryPolicy,
factory.setAdviceChain and the commented
ConditionalRejectingErrorHandler/TranscodeFatalExceptionStrategy symbols to
locate the code.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/config/TranscodeErrorHandler.java`:
- Around line 16-20: TranscodeErrorHandler.handleError currently throws
ImmediateAcknowledgeAmqpException for fatal errors which ACKs the message and
bypasses DLQ; change the behavior so fatal errors are rejected and routed to DLQ
by throwing AmqpRejectAndDontRequeueException instead of
ImmediateAcknowledgeAmqpException (i.e., when fatalExceptionStrategy.isFatal(t)
&& t instanceof ListenerExecutionFailedException, throw
AmqpRejectAndDontRequeueException with context), or alternatively persist the
message in a separate storage/queue before acknowledging; update the conditional
flow around fatalExceptionStrategy.isFatal, ListenerExecutionFailedException,
ImmediateAcknowledgeAmqpException and AmqpRejectAndDontRequeueException to
ensure unrecoverable errors are preserved for investigation and routed to the
DLQ.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/exception/TranscodeErrorCode.java`:
- Around line 16-34: Add an explicit category to TranscodeErrorCode (e.g., an
enum Category {FATAL, RETRYABLE} or a boolean isRetryable) and update every enum
constant declaration (FILE_NOT_FOUND, PROBE_FAILED, etc.) to include the correct
category instead of relying on comments; then update the base exception class
(the TranscodeException or equivalent that currently accepts a
TranscodeErrorCode) to validate the passed
TranscodeErrorCode.category/isRetryable and throw an IllegalArgumentException if
the error code's category conflicts with how the exception is being
constructed/used, ensuring retry/DLQ policies are driven by the enum metadata
rather than comments.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/processbuilder/ProcessBuilderFfmpegExecutor.java`:
- Around line 91-94: The catch block in ProcessBuilderFfmpegExecutor currently
catches IOException | InterruptedException and throws a FfmpegException without
restoring the thread interrupt state; change the error handling so that
InterruptedException is handled (or rethrown) after calling
Thread.currentThread().interrupt() to restore the interrupt flag before
creating/throwing the FfmpegException (still using
TranscodeErrorCode.FFMPEG_FAILED and including resolutionKey in the message) —
you can split the multi-catch into separate catches or detect instanceof
InterruptedException and call Thread.currentThread().interrupt() prior to
throwing.
In `@apps/transcoder/src/main/java/com/ott/transcoder/JobOrchestrator.java`:
- Around line 56-57: The call diskSpaceGuard.check(Path.of(message.originUrl()))
fails for S3 keys because Path.of only handles filesystem paths; update the
logic to detect S3 (e.g., originUrl.startsWith("s3://")) and for S3 keys perform
the same handling as videoStorage.download by resolving the local download
target or checking disk space against the local temp path that videoStorage will
use (or by using videoStorage's API to get the download target/metadata),
otherwise continue using Path.of for local paths; adjust the code around
diskSpaceGuard.check, TranscodeMessage.originUrl, and videoStorage.download to
ensure S3 keys are handled without calling Path.of.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipeline.java`:
- Around line 9-13: The CommandPipeline.execute signature no longer guarantees
required inputs (mediaId, inputFile, workDir, ProbeResult); update the contract
so implementations don’t rely on external mutable state by either (A) enriching
the Command implementation (e.g., add mediaId, Path inputFile, Path workDir,
ProbeResult to TranscodeCommand) so execute(T command) carries all data, or (B)
introduce an immutable ExecutionContext value object and change
CommandPipeline.execute to accept execute(T command, ExecutionContext ctx) (and
adjust HLSTranscodePipeline and other implementations accordingly) and update
support() usages to remain compatible; ensure the chosen approach makes fields
final/immutable and update constructors and callers to pass the context.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineExecutor.java`:
- Around line 19-24: The current type cast in invoke(CommandPipeline<T>
pipeline, Command command) relies on support() being perfect; add defensive
checks: update TranscodeCommandPipeline.support(Command command) to verify both
command instanceof TranscodeCommand and CommandType.TRANSCODE (use
TranscodeCommandPipeline.support() and TranscodeCommand/CommandType.TRANSCODE),
and in CommandPipelineExecutor.invoke(...) check that the incoming command is an
instance of the expected concrete type (or that pipeline.support(command)
returns true) before casting; if the check fails, throw a clear
IllegalArgumentException (or log and skip) to avoid a ClassCastException at
runtime.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.java`:
- Around line 19-22: The execute(TranscodeCommand) method in
TranscodeCommandPipeline is currently empty causing false-positive successes
because JobOrchestrator.handle() treats the command as completed; either
implement the real transcode invocation or, until implementation is ready, make
execute(TranscodeCommand) fail loudly (e.g., throw an
UnsupportedOperationException or a custom RuntimeException) so
JobOrchestrator.handle() will mark the job failed and prevent silent success;
locate the TranscodeCommandPipeline.execute method and add the temporary guard
(or call into the actual transcoding service/runner and propagate errors) so
failures surface instead of being ignored.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/queue/TranscodeMessage.java`:
- Around line 9-15: TranscodeMessage currently allows a null ingestJobId which
lets JobOrchestrator build ambiguous directory keys; add a compact constructor
to the TranscodeMessage record that validates mediaId, ingestJobId and originUrl
(e.g. using Objects.requireNonNull(...) and optionally check positive IDs) and
throw a clear exception (NPE/IllegalArgumentException) with a message if any are
invalid so messages without ingestJobId are rejected early.
In `@docker-compose.yml`:
- Around line 120-121: The docker-compose file currently injects an empty
FFPROBE_PATH which overrides Spring's default (transcoder.ffprobe.path:ffprobe)
and causes ProcessBuilderFfprobeExecutor (line referenced) to try running an
empty path; fix by either providing a sensible default in docker-compose (e.g.
change the env entry to FFPROBE_PATH: ${FFPROBE_PATH:-ffprobe}) or stop
exporting the FFPROBE_PATH env when it's unset so the Spring property default
applies; update the docker-compose.yml env block accordingly.
---
Outside diff comments:
In
`@apps/transcoder/src/main/java/com/ott/transcoder/inspection/DiskSpaceGuard.java`:
- Around line 20-22: The check method in DiskSpaceGuard is currently empty;
implement it to verify available disk space by (1) using Files.size(originPath)
to get the original file size, (2) multiplying by the existing multiplier field
to compute required bytes, (3) obtaining the FileStore for originPath via
Files.getFileStore(originPath) and calling getUsableSpace() to get available
bytes, and (4) if usableSpace < requiredBytes throw an explicit runtime
exception (e.g., IllegalStateException with a clear message) or a custom
InsufficientDiskSpaceException; handle IOExceptions from Files methods by
wrapping them in the same runtime exception so callers fail fast. Ensure imports
for java.nio.file.Files, java.nio.file.FileStore and java.io.IOException are
present; if this was intended as a placeholder instead, replace the empty body
with a TODO comment and throw UnsupportedOperationException from
DiskSpaceGuard.check to make the placeholder explicit.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.java`:
- Around line 75-106: parseJson currently assumes format is non-null and calls
format.has(...) which can NPE; add an explicit null check for the JsonNode
format at the start of the metadata handling in parseJson and, if null, throw a
ProbeException mapped to the same TranscodeErrorCode family used elsewhere
(e.g., throw new ProbeException(TranscodeErrorCode.<appropriate_code>, "format
metadata missing" )) so all parsing failures use the same exception mapping;
after that safe-check you can call format.has(...)/format.get(...) as before and
compute duration/fps/bitrate.
- Around line 49-71: The current implementation in ProcessBuilderFfprobeExecutor
uses process.getInputStream().readAllBytes() which can block and prevent
waitFor(2, TimeUnit.MINUTES) from ever seeing the process exit; change the
execution to consume the child process streams asynchronously before waiting:
close process.getOutputStream() immediately, start background readers (or use an
ExecutorService) to drain process.getInputStream() (and process.getErrorStream()
if you stop using redirectErrorStream(true)) into a String/ByteArray without
blocking the main thread, then call waitFor with the timeout and handle timeout
by destroyForcibly(); also in the catch for InterruptedException restore the
thread interrupt flag via Thread.currentThread().interrupt() and rethrow the
ProbeException so transient interrupts are preserved.
---
Nitpick comments:
In
`@apps/transcoder/src/main/java/com/ott/transcoder/constant/IngestJobConstant.java`:
- Around line 7-10: The nested constant classes (DirectoryConstant,
ValidateConstant, VideoConstant, AudioConstant) should not be instantiable;
change their public constructors to private no-arg constructors inside
IngestJobConstant so the classes remain utility holders for constants only
(e.g., replace the public DirectoryConstant() {} with private
DirectoryConstant() {} and apply the same change to ValidateConstant,
VideoConstant, and AudioConstant).
In
`@apps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/UnsupportedMediaException.java`:
- Around line 6-9: Add an overloaded constructor to UnsupportedMediaException
that accepts (TranscodeErrorCode errorCode, String message, Throwable cause) and
forwards all three to the superclass constructor (FatalException) so the
original cause is preserved; mirror the existing overload pattern used by
InvalidInputException and FfmpegException to restore the exception chain and
update any places that construct UnsupportedMediaException with a cause to use
the new constructor.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/FfmpegExecutor.java`:
- Around line 15-23: Update the FfmpegExecutor interface Javadoc to explicitly
document the runtime exception behavior: add an `@throws` FfmpegException line
describing that implementations (e.g., ProcessBuilderFfmpegExecutor) wrap
IO/Interrupted exceptions into FfmpegException so callers should expect a
runtime FfmpegException from execute(Path inputFile, Path outputDir,
TranscodeProfile profile); this keeps the interface consistent with the
implementation and informs callers of the possible runtime exception.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/processbuilder/ProcessBuilderFfmpegExecutor.java`:
- Around line 67-74: The current ProcessBuilderFfmpegExecutor reads
process.getInputStream() fully on the calling thread before calling
process.waitFor(), which can deadlock if FFmpeg floods stdout/err; change to
consume both stdout and stderr concurrently by starting background gobbler
threads (or use processBuilder.redirectErrorStream(true) if merging is
acceptable) immediately after processBuilder.start(), e.g., create
StreamGobbler-style Runnables for process.getInputStream() and
process.getErrorStream() (used in ProcessBuilderFfmpegExecutor around
processBuilder.start(), BufferedReader and
process.getInputStream()/getErrorStream()), start them before invoking
process.waitFor(), and ensure they are joined or completed after waitFor() to
preserve logging and resource cleanup.
In `@apps/transcoder/src/main/java/com/ott/transcoder/JobOrchestrator.java`:
- Around line 52-53: The workDir path in JobOrchestrator currently concatenates
PREFIX_WORK_DIR + mediaId + SUFFIX_WORK_DIR + ingestJobId producing strings like
"media-123job-456"; update the formatting so there is a clear separator between
mediaId and the job segment—either insert an explicit delimiter (e.g.,
PREFIX_WORK_DIR + mediaId + "-" + SUFFIX_WORK_DIR + ingestJobId) or change the
SUFFIX_WORK_DIR constant to include the leading hyphen (e.g., SUFFIX_WORK_DIR =
"-job-") so Path.of(tempDir, PREFIX_WORK_DIR + mediaId + SUFFIX_WORK_DIR +
ingestJobId) yields "media-123-job-456" for easier debugging and readability.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineFactory.java`:
- Around line 15-20: find 메서드의 orElseThrow()가 어떤 Command에 대해 파이프라인을 찾지 못했는지 정보를
제공하지 않으므로, CommandPipelineFactory.find에서
commandPipelineList.stream(...).findFirst().orElseThrow() 부분을 orElseThrow(() ->
new NoSuchElementException(...)) 형태로 바꿔 구체적인 메시지(예: "No pipeline found for
Command: " + command 또는 command.getType())를 포함해 던지도록 변경하고, 메시지에 사용될 식별자(예:
command 또는 command.getId())를 포함해 CommandPipeline.support를 호출하던 대상 Command을 명시하도록
수정하세요.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HLSTranscodePipeline.java`:
- Around line 1-55: The file HLSTranscodePipeline currently contains a fully
commented-out implementation which is confusing; either remove the file from the
tree or convert it into an explicit failing stub so its presence is unambiguous.
If you choose the stub approach, keep the class HLSTranscodePipeline and its
public API (support, execute) but implement them to immediately fail (e.g.,
throw UnsupportedOperationException or return a clear negative result) with a
log message indicating the pipeline is not implemented; ensure
constructors/fields (transcodePlanner, ffmpegExecutor, masterPlaylistGenerator,
videoStorage) are removed or replaced with no-op/explicit-failure behavior so
the class compiles and cannot be mistaken for a working pipeline.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.java`:
- Around line 14-17: support() currently only checks Command.getType() against
CommandType.TRANSCODE which allows other Command implementations to falsely
claim TRANSCODE and cause a ClassCastException in
CommandPipelineExecutor.invoke(); update
TranscodeCommandPipeline.support(Command) to verify both the type and the actual
runtime class (e.g., check command instanceof TranscodeCommand or
command.getClass() == TranscodeCommand.class) before returning true so the
executor's unchecked cast is safe.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/storage/LocalVideoStorage.java`:
- Around line 51-60: 현재 LocalVideoStorage의 Files.walk(localDir) 결과를
Stream#toList()로 전부 모으고 있어 메모리와 지연을 유발합니다; toList()를 제거하고
fileStream.filter(Files::isRegularFile).forEach(...)로 스트림을 바로 순회하도록 변경하세요; 각 파일에
대해 localDir.relativize(file), destination.resolve(relativePath),
Files.createDirectories(targetFile.getParent()), Files.copy(...) 로 기존 복사 로직을
수행하되 람다 내부에서 발생하는 IOException은 UncheckedIOException으로 래핑하거나 별도 메서드로 빼서 처리(또는
외부에서 적절히 처리)하여 컴파일/예외 처리를 맞추세요.
In
`@modules/domain/src/main/java/com/ott/domain/video_profile/domain/Resolution.java`:
- Around line 9-18: The Resolution enum mixes types: videoBitrate is a long
while audioBitrate is a String; update the enum to be consistent by changing
audioBitrate to a numeric type (e.g., long) and provide a formatting accessor
for FFmpeg (e.g., toFfmpegAudioBitrate() or getAudioBitrateFfmpeg()) that
appends the "k" suffix when needed; modify the enum constants (P360, P720,
P1080) to supply numeric audio bitrates and add the new accessor alongside
existing videoBitrate/getVideoBitrate to keep callers working.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a4909f53-9f00-43ea-abd6-9a983a61e332
📒 Files selected for processing (42)
.gitignoreapps/transcoder/src/main/java/com/ott/transcoder/JobOrchestrator.javaapps/transcoder/src/main/java/com/ott/transcoder/command/Command.javaapps/transcoder/src/main/java/com/ott/transcoder/command/CommandExtractor.javaapps/transcoder/src/main/java/com/ott/transcoder/command/CommandType.javaapps/transcoder/src/main/java/com/ott/transcoder/command/TranscodeCommand.javaapps/transcoder/src/main/java/com/ott/transcoder/config/RabbitConfig.javaapps/transcoder/src/main/java/com/ott/transcoder/config/TranscodeErrorHandler.javaapps/transcoder/src/main/java/com/ott/transcoder/config/TranscodeFatalExceptionStrategy.javaapps/transcoder/src/main/java/com/ott/transcoder/constant/IngestJobConstant.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/TranscodeErrorCode.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/TranscodeException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/FatalException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/InvalidInputException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/fatal/UnsupportedMediaException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/retryable/FfmpegException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/retryable/ProbeException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/retryable/RetryableException.javaapps/transcoder/src/main/java/com/ott/transcoder/exception/retryable/StorageException.javaapps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/.gitkeepapps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/FfmpegExecutor.javaapps/transcoder/src/main/java/com/ott/transcoder/ffmpeg/execution/processbuilder/ProcessBuilderFfmpegExecutor.javaapps/transcoder/src/main/java/com/ott/transcoder/inspection/DiskSpaceGuard.javaapps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.javaapps/transcoder/src/main/java/com/ott/transcoder/inspection/validation/FileValidator.javaapps/transcoder/src/main/java/com/ott/transcoder/inspection/validation/StreamValidator.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipeline.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineExecutor.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineFactory.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HLSTranscodePipeline.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HlsTranscodePipeline.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/MasterPlaylistGenerator.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.javaapps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodePlanner.javaapps/transcoder/src/main/java/com/ott/transcoder/queue/MessageListener.javaapps/transcoder/src/main/java/com/ott/transcoder/queue/TranscodeMessage.javaapps/transcoder/src/main/java/com/ott/transcoder/queue/rabbit/RabbitTranscodeListener.javaapps/transcoder/src/main/java/com/ott/transcoder/storage/LocalVideoStorage.javaapps/transcoder/src/main/java/com/ott/transcoder/transcode/FfmpegExecutor.javaapps/transcoder/src/main/java/com/ott/transcoder/transcode/processbuilder/ProcessBuilderFfmpegExecutor.javadocker-compose.ymlmodules/domain/src/main/java/com/ott/domain/video_profile/domain/Resolution.java
💤 Files with no reviewable changes (3)
- apps/transcoder/src/main/java/com/ott/transcoder/transcode/processbuilder/ProcessBuilderFfmpegExecutor.java
- apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/HlsTranscodePipeline.java
- apps/transcoder/src/main/java/com/ott/transcoder/transcode/FfmpegExecutor.java
Comment on lines
+16
to
+22
| public List<Command> extractCommand(TranscodeMessage message, ProbeResult probeResult) { | ||
| List<Command> commandList = new ArrayList<>(); | ||
|
|
||
| for (Resolution resolution : Resolution.values()) | ||
| commandList.add(new TranscodeCommand(resolution)); | ||
|
|
||
| // TODO ... |
There was a problem hiding this comment.
현재 추출 로직은 입력과 무관하게 모든 해상도 커맨드를 만들어 업스케일 방지 로직을 우회합니다.
message와 probeResult를 전혀 쓰지 않고 Resolution.values() 전체를 내보내면, 저해상도 원본에도 720p/1080p 커맨드가 생성됩니다. 이렇게 되면 TranscodePlanner의 필터링이 무력화되고, 이슈 목표였던 필수/선택 작업 분리도 여기서 반영할 수 없습니다. planner 결과를 기준으로 커맨드를 만들고, 선택 작업은 message에서 분기해 주세요.
제안 diff
-import com.ott.domain.video_profile.domain.Resolution;
import com.ott.transcoder.inspection.probe.ProbeResult;
+import com.ott.transcoder.pipeline.hls.TranscodePlanner;
import com.ott.transcoder.queue.TranscodeMessage;
@@
`@RequiredArgsConstructor`
`@Component`
public class CommandExtractor {
+ private final TranscodePlanner transcodePlanner;
public List<Command> extractCommand(TranscodeMessage message, ProbeResult probeResult) {
List<Command> commandList = new ArrayList<>();
- for (Resolution resolution : Resolution.values())
- commandList.add(new TranscodeCommand(resolution));
+ transcodePlanner.plan(probeResult).forEach(profile ->
+ commandList.add(new TranscodeCommand(profile.resolution())));
- // TODO ...
+ // TODO: message 기반 optional command 분기 추가
return commandList;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/command/CommandExtractor.java`
around lines 16 - 22, The current extractCommand method indiscriminately creates
a TranscodeCommand for every Resolution, ignoring message and probeResult;
instead, call the TranscodePlanner with the TranscodeMessage and ProbeResult to
get the planned resolutions/tasks and only construct TranscodeCommand instances
for those planner results (use Resolution values returned by the planner rather
than Resolution.values()), and branch optional/choice tasks based on flags in
the TranscodeMessage when creating each TranscodeCommand so optional tasks are
only included when message indicates them.
Comment on lines
+7
to
+20
| @AllArgsConstructor | ||
| @Getter | ||
| public class TranscodeCommand implements Command { | ||
|
|
||
| Resolution resolution; | ||
|
|
||
| @Override | ||
| public CommandType getType() { | ||
| return CommandType.TRANSCODE; | ||
| } | ||
|
|
||
| @Override | ||
| public String getCommandKey() { | ||
| return resolution.getKey(); |
There was a problem hiding this comment.
resolution은 생성 시점에 null을 막아 주세요.
지금 구현은 new TranscodeCommand(null)을 허용해서 실제 실패가 getCommandKey()나 이후 파이프라인 실행 시점의 NPE로 늦게 터집니다. 커맨드 생성 시점에 바로 검증하고 불변으로 고정하는 편이 안전합니다.
수정 예시
import com.ott.domain.video_profile.domain.Resolution;
-import lombok.AllArgsConstructor;
import lombok.Getter;
+import java.util.Objects;
-@AllArgsConstructor
`@Getter`
public class TranscodeCommand implements Command {
- Resolution resolution;
+ private final Resolution resolution;
+
+ public TranscodeCommand(Resolution resolution) {
+ this.resolution = Objects.requireNonNull(resolution, "resolution");
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/command/TranscodeCommand.java`
around lines 7 - 20, Make TranscodeCommand fail fast on null resolution: make
the resolution field final and remove the generated all-args constructor, then
add an explicit constructor that calls Objects.requireNonNull(resolution,
"resolution must not be null") (or throws IllegalArgumentException) so new
TranscodeCommand(null) cannot be created; this ensures methods like
getCommandKey() (which calls resolution.getKey()) cannot throw a delayed NPE.
Comment on lines
+95
to
+103
| // FatalException 발생 시 재시도 없이 즉시 거절(Reject) 처리 | ||
| // factory.setErrorHandler(new ConditionalRejectingErrorHandler(new TranscodeFatalExceptionStrategy())); | ||
| factory.setErrorHandler(errorHandler()); | ||
|
|
||
| // 일시적 장애(Retryable) 시 3회 재시도 (1s -> 2s -> 4s) | ||
| factory.setAdviceChain(RetryInterceptorBuilder.stateless() | ||
| .retryPolicy(new SimpleRetryPolicy(3, Map.of(RetryableException.class, true, FatalException.class, false), true)) | ||
| .backOffOptions(1000, 2.0, 10000) | ||
| .build()); |
There was a problem hiding this comment.
주석의 재시도 횟수와 실제 설정값이 불일치합니다.
주석에는 "3회 재시도"라고 되어 있지만, SimpleRetryPolicy(3, ...)의 첫 번째 인자는 maxAttempts이므로 실제로는 최초 1회 시도 + 2회 재시도 = 총 3회입니다. 3회 재시도를 원하신다면 maxAttempts를 4로 설정해야 합니다.
또한 Line 96의 주석 처리된 코드는 더 이상 필요하지 않다면 삭제하는 것이 좋습니다.
🔧 제안된 수정
// FatalException 발생 시 재시도 없이 즉시 거절(Reject) 처리
-// factory.setErrorHandler(new ConditionalRejectingErrorHandler(new TranscodeFatalExceptionStrategy()));
factory.setErrorHandler(errorHandler());
- // 일시적 장애(Retryable) 시 3회 재시도 (1s -> 2s -> 4s)
+ // 일시적 장애(Retryable) 시 최대 3회 시도 (1s -> 2s 간격으로 2회 재시도)
factory.setAdviceChain(RetryInterceptorBuilder.stateless()
.retryPolicy(new SimpleRetryPolicy(3, Map.of(RetryableException.class, true, FatalException.class, false), true))
.backOffOptions(1000, 2.0, 10000)
.build());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/transcoder/src/main/java/com/ott/transcoder/config/RabbitConfig.java`
around lines 95 - 103, The comment about "3회 재시도" is misleading because
SimpleRetryPolicy(3, ...) sets maxAttempts (initial attempt + 2 retries = 3
attempts total); either update the comment to reflect "총 3회(1시도+2재시도)" or change
SimpleRetryPolicy's first argument to 4 to get 3 retries (total 4 attempts);
also remove the unused commented-out factory.setErrorHandler(new
ConditionalRejectingErrorHandler(new TranscodeFatalExceptionStrategy())) line if
it's not needed to keep the file clean, referencing SimpleRetryPolicy,
factory.setAdviceChain and the commented
ConditionalRejectingErrorHandler/TranscodeFatalExceptionStrategy symbols to
locate the code.
Comment on lines
+16
to
+20
| public void handleError(Throwable t) { | ||
| if (this.fatalExceptionStrategy.isFatal(t) && t instanceof ListenerExecutionFailedException) | ||
| throw new ImmediateAcknowledgeAmqpException("[Fatal Error Detected]: " + t.getMessage(), t); | ||
| // if (t instanceof RetryableException) | ||
| throw new AmqpRejectAndDontRequeueException("[Retryable Error Detected]: " + t.getMessage(), t); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring AMQP ImmediateAcknowledgeAmqpException vs AmqpRejectAndDontRequeueException error handling behavior
💡 Result:
AmqpRejectAndDontRequeueException (ARADRE)
Intent: Reject/Nack the message and do not requeue it (so it is discarded or dead-lettered if the queue has DLX/DLQ configured). (docs.enterprise.spring.io)
What the container does:
- The container’s requeue decision logic treats ARADRE as “do not requeue” (overriding the normal
defaultRequeueRejected=truebehavior). (docs.enterprise.spring.io) - With MANUAL ack mode, ARADRE can still force a reject if it is the top-level exception and was created with
rejectManual=true(or thrown by an error handler as top-level). (docs.spring.io)
Typical use: “Poison message” / unrecoverable business error where you want DLQ (or discard) instead of infinite redeliveries.
ImmediateAcknowledgeAmqpException (IAAE)
Intent: Ack immediately (as soon as possible), no rollback, and stop consuming more messages in the current transaction/batch. (docs.spring.io)
What the container does:
- It’s a special signal to the listener container that the current batch should be acknowledged immediately, without rollback, and without consuming more messages in the current transaction. (docs.spring.io)
- The container checks the cause chain for IAAE and gives it precedence if it appears before an ARADRE in the cause chain (i.e., you can effectively “force ack” even if something else might have caused rejection later in the chain). (docs.spring.io)
Typical use: “Give up and drop/park at app level” patterns—e.g., after retries are exhausted you decide to ack to prevent any further broker redelivery (and handle compensation elsewhere).
Practical difference in one line
- ARADRE: “This message failed → reject it → don’t requeue (DLQ/discard).” (docs.enterprise.spring.io)
- IAAE: “This batch/message is done (even if ‘failed’) → ack it now → don’t roll back and don’t keep consuming in this TX/batch.” (docs.spring.io)
Citations:
- 1: https://docs.enterprise.spring.io/spring-amqp-dist/docs/3.0.16/reference/html/index.html
- 2: https://docs.enterprise.spring.io/spring-amqp-dist/docs/3.0.16/reference/html/index.html
- 3: https://docs.spring.io/spring-amqp/docs/current/api/org/springframework/amqp/AmqpRejectAndDontRequeueException.html
- 4: https://docs.spring.io/spring-amqp/docs/current/api/org/springframework/amqp/ImmediateAcknowledgeAmqpException.html
- 5: https://docs.spring.io/spring-amqp/docs/current/api/org/springframework/amqp/ImmediateAcknowledgeAmqpException.html
- 6: https://docs.spring.io/spring-amqp/docs/current/api/org/springframework/amqp/rabbit/listener/AbstractMessageListenerContainer.html
- 7: https://docs.enterprise.spring.io/spring-amqp-dist/docs/3.0.16/reference/html/index.html
- 8: https://docs.spring.io/spring-amqp/docs/current/api/org/springframework/amqp/ImmediateAcknowledgeAmqpException.html
🏁 Script executed:
# Find and examine the full TranscodeErrorHandler file
find . -name "TranscodeErrorHandler.java" -type f | head -5Repository: OpenTheTaste/backend
Length of output: 148
🏁 Script executed:
# Search for DLQ configuration or failure handling patterns in the transcoder module
rg -i "deadletter|dlq|failur|park" apps/transcoder --type java -l | head -10Repository: OpenTheTaste/backend
Length of output: 137
🏁 Script executed:
# Look for other ErrorHandler implementations or AMQP configuration in the codebase
find . -name "*ErrorHandler*.java" -o -name "*RabbitConfig*.java" -o -name "*AmqpConfig*.java" | grep -E "(java|config)" | head -10Repository: OpenTheTaste/backend
Length of output: 224
🏁 Script executed:
cat -n apps/transcoder/src/main/java/com/ott/transcoder/config/TranscodeErrorHandler.javaRepository: OpenTheTaste/backend
Length of output: 1202
🏁 Script executed:
cat -n apps/transcoder/src/main/java/com/ott/transcoder/config/RabbitConfig.java | head -80Repository: OpenTheTaste/backend
Length of output: 3742
🏁 Script executed:
# Search for failure/retry/state persistence mechanisms in transcoder
rg -i "job|state|persist|failure|retry" apps/transcoder/src/main/java --type java -A 2 | head -100Repository: OpenTheTaste/backend
Length of output: 10101
🏁 Script executed:
# Check if there's any failure tracking mechanism downstream of ErrorHandler
rg -i "immediate.*acknowledge|transcodeerrorhandler" apps/transcoder --type java -A 3 -B 1 | head -30Repository: OpenTheTaste/backend
Length of output: 2663
🏁 Script executed:
# Look for any job state update or logging happening alongside ErrorHandler usage
fd ".*Listener.*" apps/transcoder/src --type f | xargs -I {} grep -l "handleError\|@RabbitListener" {}Repository: OpenTheTaste/backend
Length of output: 156
🏁 Script executed:
# Examine the RabbitTranscodeListener to see if there's state tracking before ErrorHandler is invoked
cat -n apps/transcoder/src/main/java/com/ott/transcoder/queue/rabbit/RabbitTranscodeListener.javaRepository: OpenTheTaste/backend
Length of output: 1392
치명 경로(fatal path)가 메시지를 ACK하여 DLQ 라우팅을 우회하고 있습니다.
Spring AMQP 시맨틱에 따르면:
ImmediateAcknowledgeAmqpException: 메시지를 즉시 ACK → 브로커에서 제거 (DLQ로 라우팅되지 않음)AmqpRejectAndDontRequeueException: 메시지 거부 + 재큐 불가 → DLQ로 라우팅
현재 코드는 치명 오류에 IAAE를 던지므로 메시지가 브로커에서 완전히 삭제되고, RabbitConfig에 구성된 DLQ 인프라도 우회됩니다. 복구 불가능한 치명 오류는 조사를 위해 보존되어야 하는데, 현재는 상태 기록 없이 자동으로 손실됩니다.
ARADRE를 던져 DLQ로 라우팅하거나, ErrorHandler에서 메시지를 별도 보관 큐로 전달한 후 정상 응답을 반환하는 방식으로 변경해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/config/TranscodeErrorHandler.java`
around lines 16 - 20, TranscodeErrorHandler.handleError currently throws
ImmediateAcknowledgeAmqpException for fatal errors which ACKs the message and
bypasses DLQ; change the behavior so fatal errors are rejected and routed to DLQ
by throwing AmqpRejectAndDontRequeueException instead of
ImmediateAcknowledgeAmqpException (i.e., when fatalExceptionStrategy.isFatal(t)
&& t instanceof ListenerExecutionFailedException, throw
AmqpRejectAndDontRequeueException with context), or alternatively persist the
message in a separate storage/queue before acknowledging; update the conditional
flow around fatalExceptionStrategy.isFatal, ListenerExecutionFailedException,
ImmediateAcknowledgeAmqpException and AmqpRejectAndDontRequeueException to
ensure unrecoverable errors are preserved for investigation and routed to the
DLQ.
Comment on lines
+16
to
+34
| // ========== Fatal (T0XX) — 재시도 불가 ========== | ||
| FILE_NOT_FOUND("T001", "파일을 찾을 수 없습니다"), | ||
| FILE_NOT_READABLE("T002", "파일 읽기 권한이 없습니다"), | ||
| FILE_EMPTY("T003", "파일 크기가 0입니다"), | ||
| FILE_SIZE_EXCEEDED("T004", "파일 크기 상한을 초과했습니다"), | ||
| INVALID_FILE_FORMAT("T005", "알 수 없는 파일 포맷입니다"), | ||
| UNSUPPORTED_CODEC("T006", "지원하지 않는 코덱입니다"), | ||
| INVALID_RESOLUTION("T007", "해상도가 유효하지 않습니다"), | ||
| INVALID_DURATION("T008", "재생 시간이 유효하지 않습니다"), | ||
| INVALID_FPS("T009", "프레임레이트가 유효하지 않습니다"), | ||
| NO_VIDEO_STREAM("T010", "비디오 스트림이 없습니다"), | ||
|
|
||
| // ========== Retryable (T1XX) — 재시도 가능 ========== | ||
| PROBE_FAILED("T101", "미디어 분석에 실패했습니다"), | ||
| PROBE_TIMEOUT("T102", "미디어 분석 시간이 초과되었습니다"), | ||
| FFMPEG_FAILED("T103", "트랜스코딩에 실패했습니다"), | ||
| FFMPEG_TIMEOUT("T104", "트랜스코딩 시간이 초과되었습니다"), | ||
| STORAGE_FAILED("T105", "스토리지 작업에 실패했습니다"), | ||
| DISK_SPACE_INSUFFICIENT("T106", "디스크 공간이 부족합니다") |
There was a problem hiding this comment.
재시도 분류가 주석에만 있어 타입-코드 불일치가 막히지 않습니다.
지금은 T0XX/T1XX 구분이 설명뿐이라 예외 생성자에 fatal/retryable 코드를 섞어 넣어도 그대로 통과합니다. 현재 retry/DLQ 정책은 예외 타입으로 결정되므로 이런 불일치는 잘못된 재시도나 조기 폐기로 이어질 수 있습니다. TranscodeErrorCode에 category/retryable 필드를 모델링하고 베이스 예외에서 검증하는 쪽이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/exception/TranscodeErrorCode.java`
around lines 16 - 34, Add an explicit category to TranscodeErrorCode (e.g., an
enum Category {FATAL, RETRYABLE} or a boolean isRetryable) and update every enum
constant declaration (FILE_NOT_FOUND, PROBE_FAILED, etc.) to include the correct
category instead of relying on comments; then update the base exception class
(the TranscodeException or equivalent that currently accepts a
TranscodeErrorCode) to validate the passed
TranscodeErrorCode.category/isRetryable and throw an IllegalArgumentException if
the error code's category conflicts with how the exception is being
constructed/used, ensuring retry/DLQ policies are driven by the enum metadata
rather than comments.
Comment on lines
+9
to
+13
| public interface CommandPipeline<T extends Command> { | ||
|
|
||
| boolean support(Command command); | ||
|
|
||
| void execute(Long mediaId, Path inputFile, Path workDir, ProbeResult probeResult) throws Exception; | ||
| void execute(T command); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Related file locations =="
for pattern in \
'Command\.java$' \
'TranscodeCommand\.java$' \
'CommandPipeline\.java$' \
'CommandPipelineExecutor\.java$' \
'CommandPipelineFactory\.java$' \
'TranscodeCommandPipeline\.java$' \
'HLSTranscodePipeline\.java$'
do
fd "$pattern" apps/transcoder/src/main/java 2>/dev/null || true
done
echo
echo "== Command interface and implementations =="
for pattern in \
'Command\.java$' \
'TranscodeCommand\.java$'
do
fd "$pattern" apps/transcoder/src/main/java 2>/dev/null | while read -r f; do
echo "===== $f ====="
head -100 "$f"
done
done
echo
echo "== CommandPipeline interface and implementations =="
for pattern in \
'CommandPipeline\.java$' \
'TranscodeCommandPipeline\.java$' \
'HLSTranscodePipeline\.java$'
do
fd "$pattern" apps/transcoder/src/main/java 2>/dev/null | while read -r f; do
echo "===== $f ====="
head -150 "$f"
done
done
echo
echo "== Execution context search =="
rg -n --type=java 'mediaId|inputFile|workDir|probeResult|ExecutionContext|set.*Context|set.*WorkDir|set.*InputFile' apps/transcoder/src/main/java/com/ott/transcoder 2>/dev/null | head -50 || trueRepository: OpenTheTaste/backend
Length of output: 14148
Command 계약만으로는 실행에 필요한 컨텍스트가 보장되지 않습니다.
이전 구현(HLSTranscodePipeline 주석 처리된 코드)은 void execute(Long mediaId, Path inputFile, Path workDir, ProbeResult probeResult)로 명시적으로 mediaId, inputFile, workDir, ProbeResult를 전달했습니다. 그러나 새로운 인터페이스는 void execute(T command)로 축약되었고, TranscodeCommand는 Resolution만 보유합니다.
이는 다음 문제를 야기합니다:
- 실행에 필요한
mediaId,inputFile,workDir,ProbeResult가 계약상 보장되지 않음 - 구현체가 이들 데이터에 접근하려면 외부 mutable state나 서비스 호출에 의존해야 함
- 동시성 안정성과 테스트 용이성이 급격히 악화됨
해결책: 실행 컨텍스트를 Command 구현체 내부에 포함시키거나(예: TranscodeCommand에 mediaId, inputFile 등 추가), 별도의 immutable ExecutionContext 객체를 함께 전달하도록 인터페이스를 개선하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipeline.java`
around lines 9 - 13, The CommandPipeline.execute signature no longer guarantees
required inputs (mediaId, inputFile, workDir, ProbeResult); update the contract
so implementations don’t rely on external mutable state by either (A) enriching
the Command implementation (e.g., add mediaId, Path inputFile, Path workDir,
ProbeResult to TranscodeCommand) so execute(T command) carries all data, or (B)
introduce an immutable ExecutionContext value object and change
CommandPipeline.execute to accept execute(T command, ExecutionContext ctx) (and
adjust HLSTranscodePipeline and other implementations accordingly) and update
support() usages to remain compatible; ensure the chosen approach makes fields
final/immutable and update constructors and callers to pass the context.
Comment on lines
+19
to
+24
| @SuppressWarnings("unchecked") | ||
| private <T extends Command> void invoke(CommandPipeline<T> pipeline, Command command) { | ||
| // 여기서 (T) 캐스팅은 팩토리가 이미 검증했으므로 | ||
| // 자바가 타입을 딱 맞춰줘서 파이프라인의 execute(T command)가 바로 호출 | ||
| pipeline.execute((T) command); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the CommandPipeline interface and related files
find . -type f -name "*.java" | xargs grep -l "interface CommandPipeline" | head -5Repository: OpenTheTaste/backend
Length of output: 144
🏁 Script executed:
# Search for TranscodeCommandPipeline class
find . -type f -name "*.java" | xargs grep -l "class TranscodeCommandPipeline" | head -5Repository: OpenTheTaste/backend
Length of output: 157
🏁 Script executed:
# Search for all support() method implementations in pipeline-related files
rg "boolean support\(Command" --type java -A 5Repository: OpenTheTaste/backend
Length of output: 1913
🏁 Script executed:
# Find the factory.find() implementation
rg "factory\.find\(" --type java -B 3 -A 5Repository: OpenTheTaste/backend
Length of output: 1115
🏁 Script executed:
# Read CommandPipeline interface fully
cat -n ./apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipeline.javaRepository: OpenTheTaste/backend
Length of output: 419
🏁 Script executed:
# Read CommandPipelineExecutor completely
cat -n ./apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineExecutor.javaRepository: OpenTheTaste/backend
Length of output: 990
🏁 Script executed:
# Find CommandPipelineFactory
find . -type f -name "*CommandPipelineFactory*"Repository: OpenTheTaste/backend
Length of output: 151
🏁 Script executed:
# Find all CommandPipeline implementations
rg "implements CommandPipeline" --type java -B 1 -A 2Repository: OpenTheTaste/backend
Length of output: 1047
🏁 Script executed:
# Read CommandPipelineFactory fully
cat -n ./apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineFactory.javaRepository: OpenTheTaste/backend
Length of output: 760
🏁 Script executed:
# Read TranscodeCommandPipeline to see complete implementation
cat -n ./apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.javaRepository: OpenTheTaste/backend
Length of output: 893
🏁 Script executed:
# Check if there are other CommandPipeline implementations
find . -type f -name "*.java" -path "*/pipeline/*" | xargs grep -l "implements CommandPipeline"Repository: OpenTheTaste/backend
Length of output: 247
🏁 Script executed:
# Check Command and CommandType to understand the structure better
find . -type f -name "Command.java" -o -name "CommandType.java" | head -5Repository: OpenTheTaste/backend
Length of output: 211
🏁 Script executed:
# Read Command interface/class
find . -type f -name "Command.java" | xargs cat -n 2>/dev/null | head -30Repository: OpenTheTaste/backend
Length of output: 234
🏁 Script executed:
# Check if TranscodeCommand sets CommandType internally
find . -type f -name "TranscodeCommand.java" | xargs cat -n 2>/dev/nullRepository: OpenTheTaste/backend
Length of output: 662
타입 안전성 주의: support() 메서드의 부분적 검증
support() 메서드가 CommandType enum만 검증하고 실제 Command 구현체 타입은 확인하지 않으므로, 방어적 프로그래밍 관점에서 instanceof 체크 추가를 권장합니다. 현재는 TranscodeCommand.getType()이 정확히 CommandType.TRANSCODE를 반환하고 파이프라인이 하나뿐이어서 문제가 없지만, 향후 동일 CommandType에 여러 파이프라인이 추가되거나 Command 구현체가 잘못된 타입을 반환하는 경우 ClassCastException이 발생할 수 있습니다.
// TranscodeCommandPipeline.support()에 instanceof 체크 추가 권장
`@Override`
public boolean support(Command command) {
return command instanceof TranscodeCommand &&
CommandType.TRANSCODE.equals(command.getType());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/CommandPipelineExecutor.java`
around lines 19 - 24, The current type cast in invoke(CommandPipeline<T>
pipeline, Command command) relies on support() being perfect; add defensive
checks: update TranscodeCommandPipeline.support(Command command) to verify both
command instanceof TranscodeCommand and CommandType.TRANSCODE (use
TranscodeCommandPipeline.support() and TranscodeCommand/CommandType.TRANSCODE),
and in CommandPipelineExecutor.invoke(...) check that the incoming command is an
instance of the expected concrete type (or that pipeline.support(command)
returns true) before casting; if the check fails, throw a clear
IllegalArgumentException (or log and skip) to avoid a ClassCastException at
runtime.
Comment on lines
+19
to
+22
| @Override | ||
| public void execute(TranscodeCommand command) { | ||
| // TODO: | ||
| } |
There was a problem hiding this comment.
execute()가 비어 있어서 작업이 성공으로 끝나도 실제 트랜스코딩은 수행되지 않습니다.
현재 JobOrchestrator.handle()은 각 command마다 이 메서드를 호출한 뒤 바로 성공 로그를 남기고 작업 디렉터리까지 정리합니다. 그래서 지금 상태로는 메시지가 정상 소비되어도 산출물 없이 성공 처리되는 false-positive가 됩니다. 실제 구현을 연결하기 전이라면 최소한 실패로 올려서 조용한 성공을 막아야 합니다.
임시 안전장치 예시
`@Override`
public void execute(TranscodeCommand command) {
- // TODO:
+ throw new IllegalStateException("TranscodeCommandPipeline.execute is not implemented yet");
}📝 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
| @Override | |
| public void execute(TranscodeCommand command) { | |
| // TODO: | |
| } | |
| `@Override` | |
| public void execute(TranscodeCommand command) { | |
| throw new IllegalStateException("TranscodeCommandPipeline.execute is not implemented yet"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/transcoder/src/main/java/com/ott/transcoder/pipeline/hls/TranscodeCommandPipeline.java`
around lines 19 - 22, The execute(TranscodeCommand) method in
TranscodeCommandPipeline is currently empty causing false-positive successes
because JobOrchestrator.handle() treats the command as completed; either
implement the real transcode invocation or, until implementation is ready, make
execute(TranscodeCommand) fail loudly (e.g., throw an
UnsupportedOperationException or a custom RuntimeException) so
JobOrchestrator.handle() will mark the job failed and prevent silent success;
locate the TranscodeCommandPipeline.execute method and add the temporary guard
(or call into the actual transcoding service/runner and propagate errors) so
failures surface instead of being ignored.
Comment on lines
9
to
15
| public record TranscodeMessage( | ||
|
|
||
| Long mediaId, | ||
| Long ingestJobId, | ||
| String originUrl | ||
| ) { | ||
| } |
There was a problem hiding this comment.
ingestJobId는 여기서 필수값으로 강제하는 편이 안전합니다.
JobOrchestrator가 이 값을 작업 디렉터리 키로 바로 사용하므로, 누락된 메시지가 들어오면 서로 다른 작업이 같은 ..._null 경로를 공유할 수 있습니다. record compact constructor로 mediaId/ingestJobId/originUrl를 즉시 검증해 두는 편이 안전합니다.
제안 코드
package com.ott.transcoder.queue;
+import java.util.Objects;
+
/**
* 트랜스코딩 요청 메시지 DTO.
*
* `@param` mediaId 트랜스코딩 대상 미디어 ID (Contents 또는 ShortForm의 media_id)
* `@param` originUrl 원본 영상 위치 (로컬 경로 또는 S3 key)
@@
public record TranscodeMessage(
Long mediaId,
Long ingestJobId,
String originUrl
) {
+ public TranscodeMessage {
+ Objects.requireNonNull(mediaId, "mediaId");
+ Objects.requireNonNull(ingestJobId, "ingestJobId");
+ Objects.requireNonNull(originUrl, "originUrl");
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/transcoder/src/main/java/com/ott/transcoder/queue/TranscodeMessage.java`
around lines 9 - 15, TranscodeMessage currently allows a null ingestJobId which
lets JobOrchestrator build ambiguous directory keys; add a compact constructor
to the TranscodeMessage record that validates mediaId, ingestJobId and originUrl
(e.g. using Objects.requireNonNull(...) and optionally check positive IDs) and
throw a clear exception (NPE/IllegalArgumentException) with a message if any are
invalid so messages without ingestJobId are rejected early.
Comment on lines
+120
to
+121
| TRANSCODER_FFPROBE_ENGINE: ${TRANSCODER_FFPROBE_ENGINE} | ||
| FFPROBE_PATH: ${FFPROBE_PATH} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ffprobe 관련 설정 참조 =="
rg -n -C2 'FFPROBE_PATH|TRANSCODER_FFPROBE_ENGINE|transcoder\.ffprobe\.(engine|path)|ffprobe' \
docker-compose.yml \
apps/transcoder/src/main/resources/application.yml \
apps/transcoder/src/main/java/com/ott/transcoder/inspection/probe/execution/processbuilder/ProcessBuilderFfprobeExecutor.java \
apps/transcoder/Dockerfile 2>/dev/null || echo "Some files not found or no matches"
echo
echo "== env 템플릿 / 문서 정의 여부 =="
while IFS= read -r file; do
echo "Checking: $file"
rg -n -C1 'FFPROBE_PATH|TRANSCODER_FFPROBE_ENGINE' "$file" 2>/dev/null || echo "No matches in $file"
done < <(fd -HI '(^|/)(\.env(\..*)?|.*env\.example|README\.md)$' . 2>/dev/null || echo "")
echo
echo "== Docker/Compose 파일 목록 =="
find . -maxdepth 2 -name "docker-compose*" -o -name ".env*" 2>/dev/null | head -20Repository: OpenTheTaste/backend
Length of output: 7620
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .env 파일 및 템플릿 검색 =="
fd -HI '(\.env|env\.example|README)' . 2>/dev/null | head -30
echo
echo "== .env 파일 내용 (FFPROBE_PATH 관련) =="
fd -HI '\.env(\..*)?$' . 2>/dev/null | while read -r file; do
echo "=== $file ==="
cat "$file" 2>/dev/null | grep -i 'ffprobe\|ffmpeg' || echo "(ffprobe/ffmpeg 관련 항목 없음)"
done
echo
echo "== README에서 FFPROBE_PATH 문서화 여부 =="
fd -HI 'README' . 2>/dev/null | while read -r file; do
if grep -qi 'ffprobe_path\|ffprobe.*path' "$file" 2>/dev/null; then
echo "Found in $file:"
grep -ni 'ffprobe' "$file" | head -10
fi
done
echo
echo "== Dockerfile의 ffprobe 설치 여부 =="
find . -name "Dockerfile*" -type f 2>/dev/null | while read -r file; do
echo "=== $file ==="
if grep -qi 'ffprobe\|ffmpeg' "$file" 2>/dev/null; then
grep -ni 'ffprobe\|ffmpeg' "$file"
else
echo "(ffprobe/ffmpeg 설치 항목 없음)"
fi
doneRepository: OpenTheTaste/backend
Length of output: 389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== transcoder/Dockerfile 전체 내용 =="
if [ -f ./apps/transcoder/Dockerfile ]; then
cat ./apps/transcoder/Dockerfile
else
echo "File not found"
fi
echo
echo "== 모든 쉘 스크립트에서 ffprobe 참조 =="
find . -name "*.sh" -type f 2>/dev/null | while read -r file; do
if grep -qi 'ffprobe' "$file" 2>/dev/null; then
echo "Found in $file:"
grep -ni 'ffprobe' "$file"
fi
done
echo
echo "== 모든 문서에서 ffprobe 설치 언급 =="
find . -name "*.md" -type f 2>/dev/null | while read -r file; do
if grep -qi 'ffprobe\|ffmpeg.*install' "$file" 2>/dev/null; then
echo "=== $file ==="
grep -ni 'ffprobe\|ffmpeg' "$file" | head -20
fi
doneRepository: OpenTheTaste/backend
Length of output: 414
FFPROBE_PATH를 기본값 없이 전달하면 런타임 실패가 발생합니다.
docker-compose.yml의 121번 줄 FFPROBE_PATH: ${FFPROBE_PATH}는 호스트 환경에 해당 변수가 없을 때 빈 문자열을 컨테이너에 주입합니다. 그러면 ProcessBuilderFfprobeExecutor.java:33의 @Value("${transcoder.ffprobe.path:ffprobe}") 기본값이 적용되지 않는데, Spring이 프로퍼티가 설정되었다고 인식하기 때문입니다(값이 비어있어도). 결과적으로 빈 경로를 실행하려다 실패합니다. 추가로 transcoder/Dockerfile에는 ffprobe가 설치되지 않으므로, 컨테이너 내 PATH에도 없습니다. 이 변수에 기본값을 설정하거나, 명시적으로 정의하지 않은 경우 환경 변수 자체를 전달하지 않아야 합니다.
수정 예시
TRANSCODER_FFPROBE_ENGINE: ${TRANSCODER_FFPROBE_ENGINE}
- FFPROBE_PATH: ${FFPROBE_PATH}
+ FFPROBE_PATH: ${FFPROBE_PATH:-ffprobe}📝 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
| TRANSCODER_FFPROBE_ENGINE: ${TRANSCODER_FFPROBE_ENGINE} | |
| FFPROBE_PATH: ${FFPROBE_PATH} | |
| TRANSCODER_FFPROBE_ENGINE: ${TRANSCODER_FFPROBE_ENGINE} | |
| FFPROBE_PATH: ${FFPROBE_PATH:-ffprobe} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 120 - 121, The docker-compose file currently
injects an empty FFPROBE_PATH which overrides Spring's default
(transcoder.ffprobe.path:ffprobe) and causes ProcessBuilderFfprobeExecutor (line
referenced) to try running an empty path; fix by either providing a sensible
default in docker-compose (e.g. change the env entry to FFPROBE_PATH:
${FFPROBE_PATH:-ffprobe}) or stop exporting the FFPROBE_PATH env when it's unset
so the Spring property default applies; update the docker-compose.yml env block
accordingly.
This was referenced Mar 14, 2026
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
2 participants
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.
📝 작업 내용
📷 스크린샷
☑️ 체크 리스트
#️⃣ 연관된 이슈
closes #92
💬 리뷰 요구사항
1. 아직 미완입니다.
2. 예외를 나눴습니다.
Summary by CodeRabbit
릴리스 노트
New Features
Chores