🔀 :: [#819] - 애플리케이션 비동기 작업에도 락이 적용되도록 수정 - #820
Merged
Merged
Conversation
Closed
Contributor
Walkthrough애플리케이션 생성·수정·배포 이벤트의 직접적인 클론, 이미지, 컨테이너, 볼륨 처리를 Changes애플리케이션 리프레시 흐름
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CreateOrDeployUseCase
participant RefreshApplicationService
participant ApplicationRemoteRepoPort
participant ContainerPort
participant DeleteApplicationDirectoryService
CreateOrDeployUseCase->>RefreshApplicationService: refresh(application)
RefreshApplicationService->>ApplicationRemoteRepoPort: cloneApplicationRemoteRepo(application)
RefreshApplicationService->>ContainerPort: buildImage(application)
RefreshApplicationService->>ContainerPort: createContainer(application, volumeMounts)
RefreshApplicationService->>DeleteApplicationDirectoryService: deleteApplicationDirectory(application)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/kotlin/com/dcd/server/core/domain/application/service/impl/RefreshApplicationServiceImpl.kt (1)
25-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
else -> {}분기에 의도를 명시하는 코멘트를 추가하는 것을 권장합니다.
SPRING_BOOT,NEST_JS,GIN타입만 원격 저장소 클론을 수행하고 다른 타입은 생략합니다. 이것이 의도적이라면, 어떤 타입이 클론이 불필요한지(예: 정적 사이트, DB 등) 간단한 코멘트로 명시하면 유지보수에 도움이 됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/dcd/server/core/domain/application/service/impl/RefreshApplicationServiceImpl.kt` around lines 25 - 30, In the when expression within RefreshApplicationServiceImpl, add a concise comment to the else branch explaining that cloning is intentionally skipped for application types that do not require a remote repository, such as static sites or databases.src/main/kotlin/com/dcd/server/core/domain/application/usecase/DeployApplicationUseCase.kt (1)
79-90: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
suspend fun내부의runBlocking은 IO 스레드를 블로킹합니다.
deployApplication은 이미suspend이며launch { ... }와 채널 소비 코루틴에서 호출됩니다. 여기서runBlocking을 사용하면 해당 코루틴이 점유한Dispatchers.IO스레드를 완료까지 블로킹하여, 동시 처리(1..3 워커) 효과를 떨어뜨리고 스레드 풀 고갈 위험이 있습니다. suspend 컨텍스트를 그대로 사용하도록runBlocking을 제거하는 것을 권장합니다.♻️ 제안 변경
private suspend fun deployApplication(application: Application) { - runBlocking { - containerPort.execute { - deleteContainer(application) - deleteImage(application) - } - - refreshApplicationService.refresh(application) - - eventPublisher.publishEvent(ChangeApplicationStatusEvent(ApplicationStatus.STOPPED, application)) - } + containerPort.execute { + deleteContainer(application) + deleteImage(application) + } + + refreshApplicationService.refresh(application) + + eventPublisher.publishEvent(ChangeApplicationStatusEvent(ApplicationStatus.STOPPED, application)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/dcd/server/core/domain/application/usecase/DeployApplicationUseCase.kt` around lines 79 - 90, deployApplication 내부의 runBlocking이 이미 suspend 함수인 실행 흐름을 불필요하게 블로킹합니다. deployApplication에서 runBlocking을 제거하고 containerPort.execute, refreshApplicationService.refresh, eventPublisher.publishEvent를 현재 suspend 컨텍스트에서 순차 호출하도록 수정하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/kotlin/com/dcd/server/core/domain/application/service/impl/RefreshApplicationServiceImpl.kt`:
- Around line 23-41: refresh 내부의 예외가 애플리케이션을 PENDING 상태로 남기고 호출자에서 처리되지 않는 문제를
수정하세요. RefreshApplicationServiceImpl의 refresh에서 저장소 복제, 이미지 생성, 컨테이너 생성 및 정리 작업을
try-catch로 감싸고, 실패 시 적절한 실패 상태 전환 이벤트를 발행하거나 상태를 복구한 뒤 예외를 다시 전달하세요. 또한
CreateApplicationUseCase, UpdateApplicationUseCase, ApplicationEventListener의
launch 블록이 실패를 관찰하고 처리할 수 있도록 예외 처리 흐름을 보완하세요.
---
Nitpick comments:
In
`@src/main/kotlin/com/dcd/server/core/domain/application/service/impl/RefreshApplicationServiceImpl.kt`:
- Around line 25-30: In the when expression within
RefreshApplicationServiceImpl, add a concise comment to the else branch
explaining that cloning is intentionally skipped for application types that do
not require a remote repository, such as static sites or databases.
In
`@src/main/kotlin/com/dcd/server/core/domain/application/usecase/DeployApplicationUseCase.kt`:
- Around line 79-90: deployApplication 내부의 runBlocking이 이미 suspend 함수인 실행 흐름을
불필요하게 블로킹합니다. deployApplication에서 runBlocking을 제거하고 containerPort.execute,
refreshApplicationService.refresh, eventPublisher.publishEvent를 현재 suspend
컨텍스트에서 순차 호출하도록 수정하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6a1c9477-5d92-47a4-8193-756f8b97dec3
📒 Files selected for processing (6)
src/main/kotlin/com/dcd/server/core/domain/application/event/listener/ApplicationEventListener.ktsrc/main/kotlin/com/dcd/server/core/domain/application/service/RefreshApplicationService.ktsrc/main/kotlin/com/dcd/server/core/domain/application/service/impl/RefreshApplicationServiceImpl.ktsrc/main/kotlin/com/dcd/server/core/domain/application/usecase/CreateApplicationUseCase.ktsrc/main/kotlin/com/dcd/server/core/domain/application/usecase/DeployApplicationUseCase.ktsrc/main/kotlin/com/dcd/server/core/domain/application/usecase/UpdateApplicationUseCase.kt
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
작업내용
체크리스트
기타
LockAspect가 비동기를 커버하도록 수정할 수 없기때문에, 비동기로 실행되는 작업중 락이 필요한 부분을 서비스로 분리해서 해당 서비스에 Lock을 적용하는 방식으로 수정Summary by CodeRabbit