-
Notifications
You must be signed in to change notification settings - Fork 0
비동기 Executor 안정화 및 AI 연동 실패 처리 전환 #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ckdals4600
wants to merge
2
commits into
main
Choose a base branch
from
feature/#256-async-executor-stabilization
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
9 changes: 9 additions & 0 deletions
9
src/main/java/com/sofa/linkiving/infra/feign/EmptyAiResponseException.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package com.sofa.linkiving.infra.feign; | ||
|
|
||
| import com.sofa.linkiving.global.error.exception.BusinessException; | ||
|
|
||
| public class EmptyAiResponseException extends BusinessException { | ||
| public EmptyAiResponseException() { | ||
| super(ExternalApiErrorCode.EXTERNAL_API_INVALID_RESPONSE); | ||
| } | ||
| } |
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CallerRunsPolicy자체가 잘못된 건 아니지만, 현재처럼 AI 연동이나 후속 이벤트 처리까지 같은 전역 async executor에 태우는 구조에서는 포화 시 비동기 작업이 요청/이벤트 스레드에서 직접 실행될 수 있다는 점이 조금 우려됩니다.이 작업들이 사용자 응답 시간과 분리되어야 하는 성격이라면, 워크로드별로 executor를 분리하거나 포화 시에는 명시적으로 reject/관측 가능하게 처리하는 쪽이 운영상 더 예측 가능할 것 같습니다.
만약
CallerRunsPolicy를 유지하는 게 의도라면, 포화 시에도 호출 스레드 실행을 허용해도 괜찮은 이유가 함께 있으면 좋겠습니다.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 지적 감사합니다.
먼저
CallerRunsPolicy를 선택한 이유는 해당executor에 올라가는 작업들의 성격때문입니다.현재 이 전역 async executor에 태우는 작업들(링크 동기화, 요약 큐 적재, 상태 알림 등)은 대부분 유실되면 안 되는 정합성 작업입니다.
특히 이들은 대부분
@TransactionalEventListener(AFTER_COMMIT)에서 트리거되는데, 여기서AbortPolicy로 드롭되면 트랜잭션은 이미 커밋됐는데 후속 작업만 유실되어 정합성이 깨져 동기화 누락이나 요약 영구 미실행 등의 문제가 발생할 수 있습니다.게다가 해당
rejection은submit시점에 호출 스레드로 던져져 리스너의@Retryable로도 잡기 불가능합니다.이러한 이유로 포화 시 작업을 잃기보다 프로듀서를 느리게 만드는 백프레셔로
CallerRunsPolicy를 택했습니다.또한 큐의 경우도
bounded(100)으로 설정하여 무한정 쌓이지 않고, 각 설정 값에 대해서도 아래와 같은 의 보수적sizing이라caller-runs발동은 지속 포화 상황의 edge 케이스라고 생각됩니다.다만 말씀하신 워크로드 혼재와 포화 시 요청/이벤트 스레드 지연 문제에 대해서는 충분히 발생 가능한 문제로 확인되어 아래와 같은 방법을 통해 보완하도록 하겠습니다.
applicationTaskExecutor는 자동계측되어executor_active_threads/executor_queued_tasks/executor_pool_size_threads원본 지표가/actuator/prometheus에 노출됩니다. 다만 이걸 보는 대시보드 패널과 포화 알림 룰은 존재하지 않아, executor 포화 패널과 알림 룰(queued가 capacity(100)에·active가 max(20)에 근접)을 이번 PR에 함께 작업해서 올리도록 하겠습니다.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다. 그렇게 하시죠.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Goder-0 말씀 주신 포화 관측 작업 진행했고, PR에도 반영했습니다.
AsyncExecutorQueueSaturation,AsyncExecutorPoolExhausted,for: 2m)이제 CallerRunsPolicy가 발동하는 포화 상황을 그래프·알림으로 사전 감지할 수 있습니다.
워크로드별 executor 분리는 이번 PR 범위와 성격이 달라 별도 이슈(#260)로 분리했습니다.