From 39d1d08f33ca8f108f84c9905d42bd2ad9bc2636 Mon Sep 17 00:00:00 2001 From: toki Date: Sun, 14 Jun 2026 21:37:34 +0900 Subject: [PATCH] =?UTF-8?q?feat(node):=20FIFO=20admission=20queue=20?= =?UTF-8?q?=EA=B5=AC=ED=98=84=20=EB=B0=8F=20concurrency=20=EC=B2=98?= =?UTF-8?q?=EB=A6=AC=20=EA=B0=9C=EC=84=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Node runner manager에서 FIFO 큐 기반 admission queue 구현 - Concurrency limit 적용으로 최대 동시 실행 job 수 제한 - Store 계층에 queue 상태 영구화 support 추가 - 관련 테스트 및 integration 테스트 업데이트 - Cloud/G06 문서 분류 조정 (archive로 이동) --- .../code_review_cloud_G06_0.log | 180 ++++ .../code_review_cloud_G06_1.log | 256 ++++++ .../code_review_cloud_G06_2.log | 202 +++++ .../04+03_fifo_admission_queue/complete.log | 54 ++ .../plan_cloud_G06_0.log} | 0 .../plan_cloud_G06_1.log | 107 +++ .../plan_cloud_G06_2.log | 86 ++ .../CODE_REVIEW-cloud-G06.md | 127 --- apps/node/internal/node/node.go | 216 +++-- .../node/node_concurrency_integration_test.go | 32 +- apps/node/internal/node/node_test.go | 837 ++++++++++++++---- apps/node/internal/node/run_manager.go | 347 +++++++- apps/node/internal/store/store.go | 10 + 13 files changed, 2015 insertions(+), 439 deletions(-) create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_1.log create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/complete.log rename agent-task/{m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/PLAN-cloud-G06.md => archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_0.log} (100%) create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_1.log create mode 100644 agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log delete mode 100644 agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/CODE_REVIEW-cloud-G06.md diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log new file mode 100644 index 0000000..fde9c99 --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log @@ -0,0 +1,180 @@ + + +# Code Review Reference - REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. + +## 개요 + +date=2026-06-14 +task=m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue, plan=0, tag=REFACTOR + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Task ids: + - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. + - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. + - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REFACTOR-1] FIFO Admission Manager | [x] | +| [REFACTOR-2] OnRunRequest Admission Path | [x] | +| [REFACTOR-3] Queue Rejection Events | [x] | + +## 구현 체크리스트 + +- [x] predecessor `03_queue_capability_contract`의 `complete.log`가 active 또는 archive에 있는지 확인한다. (archive `agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/03_queue_capability_contract/complete.log` 존재 확인. PASS 시에만 complete.log 작성 + archive 이동되는 워크플로 규칙상 존재 자체가 PASS 근거이며, predecessor가 추가한 `runtime.Capabilities`의 `MaxQueue`/`QueueTimeoutMS`/`RequestTimeoutMS` 필드를 본 구현이 직접 소비한다.) +- [x] `permitManager` 또는 새 admission manager를 FIFO queue semantics로 교체한다. (`permitManager`/`atomic.Int32`를 제거하고 FIFO-fair `fifoGate` + `admissionManager`/`admissionTicket`로 교체.) +- [x] `OnRunRequest`가 capacity 여유가 있으면 즉시 실행하고, capacity가 차면 provider별 FIFO queue에 요청을 대기시킨다. +- [x] 실행 완료, 실패, 취소, timeout 시 permit을 release하고 queue head를 실행 슬롯으로 승격한다. (`run` defer `ticket.release()` → `fifoGate.release()`가 FIFO head를 승격.) +- [x] `max_queue` 초과와 `queue_timeout_ms` 초과는 store status와 RunEvent error에 명확한 reason을 남긴다. (`reasonQueueFull`/`reasonQueueTimeout`/`reasonConcurrencyUnavailable`; store status `rejected` + RunEvent error에 동일 reason text.) +- [x] foreground/background run 모두 같은 admission path를 사용한다. (`launch()` 단일 경로. foreground는 동기, queued background는 enqueue 후 반환하고 goroutine에서 wait+execute.) +- [x] node unit/integration tests를 FIFO 정책에 맞게 추가/수정한다. +- [x] 최종 검증 명령을 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G06_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G06_M.log`로 아카이브한다. +- [ ] PASS이면 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리를 archive로 이동한다. +- [ ] PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 완료 이벤트 메타데이터를 보고하고 roadmap을 직접 수정하지 않는다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] FAIL이고 user-review gate가 트리거되지 않았으므로 다음 active `PLAN-cloud-G06.md`와 `CODE_REVIEW-cloud-G06.md`를 작성하고 `complete.log`를 작성하지 않는다. + +## 계획 대비 변경 사항 + +- **admit 단일 호출 → enqueue/wait 2단계 분리.** 계획 REFACTOR-2의 의사코드는 `admission.admit(ctx)` 단일 blocking 호출이었으나, 그대로 구현하면 queued background run이 `OnRunRequest` 안에서 승격될 때까지 caller(transport handler)를 막아 "enqueue 성공 후 반환" 요구를 위반하고 첫 통합 실행에서 deadlock이 발생했다(확인됨). 그래서 `admissionManager`를 non-blocking `enqueue()`(overflow만 즉시 판정) + blocking `admissionTicket.wait()`로 분리했다. foreground는 동기 wait, queued background는 enqueue 후 반환하고 goroutine에서 wait+execute한다. +- **store status 흐름에 `queued` 상태 추가.** 계획 REFACTOR-2 체크리스트의 "store status는 queued/running/rejected/completed 흐름이 추적 가능해야 한다. store schema 변경 없이 status string 업데이트로 가능한지 우선 확인"에 따라, schema 변경 없이 status string만으로 처리했다. 요청을 먼저 `queued`로 insert하고, 승격 시 `MarkRunning`으로 `running` 전환, reject 시 `rejected` 전환한다. 이를 위해 `store.Store`에 `MarkRunning(ctx, runID)` 1개 메서드만 추가했다(기존 `InsertRun`/`CompleteRun`은 그대로 사용). +- **`max_queue` 의미 확정.** `caps.MaxQueue`를 `>0`=큐 길이 상한, `==0`=큐 불가(가득 차면 즉시 queue_full reject, 즉 기존 reject-on-exceed와 동일), `<0`=무제한으로 정의했다. 노드 전역 global gate는 `-1`(무제한)로 구성해 per-provider 큐 정책만 reject를 좌우하게 했다. +- **`ProviderSnapshot.Queued`는 0 유지.** 계획 "제외" 항목(PLAN line 73)대로 queued snapshot 채우기는 후속 `05+04_queue_observe_snapshot` 범위라 capabilities 응답의 `queued`/`Queued`는 0으로 둔다. `fifoGate.queuedCount()` API는 05에서 쓰도록 미리 두었다. + +## 주요 설계 결정 + +- **`fifoGate` = FIFO-fair counting semaphore.** mutex + `inFlight` + `[]chan struct{}` waiter 큐. `reserve()`는 non-blocking으로 즉시 슬롯/대기채널/overflow를 결정, `waitOn(ctx, ch, applyTimeout)`은 promotion/queue_timeout/ctx-cancel을 select로 처리. `release()`는 waiter가 있으면 head를 close해 슬롯을 그대로 이전(FIFO 승격), 없으면 `inFlight--`. +- **goroutine/leak 방지.** 대기 중 timeout/cancel 시 `abandon(ch)`이 waiter 슬라이스에서 제거하고, promotion과의 race는 `abandon` 반환값으로 구분(이미 승격됐으면 슬롯을 honor). run의 `defer ticket.release()`가 success/failure/cancel/timeout 모든 종료 경로에서 슬롯을 반납. race detector(`go test -race`) clean. +- **2-gate 합성 순서.** adapter gate를 먼저 reserve(per-provider FIFO 보장, cross-adapter head-of-line blocking 회피), 그다음 global gate. `wait()`는 adapter→global 순으로 대기하고 실패 시 보유 슬롯을 unwind. queue timeout은 두 wait에 각각 적용(global gate는 무제한/무timeout이라 실질 bottleneck은 adapter gate). +- **`ErrConcurrencyLimitExceeded` 호환 유지.** 새 `admissionError`가 `Is(ErrConcurrencyLimitExceeded)`를 만족하도록 구현해 기존 `errors.Is` 호출부/테스트를 깨지 않으면서, 구체 reason(`queue_full`/`queue_timeout`/`concurrency_unavailable`)을 `admissionRejectReason`/`admissionMessage`로 노출한다. RunEvent error와 store error에 동일 reason text가 들어간다. +- **cancel-while-queued 지원.** run을 enqueue 직후 `runManager`에 register하고, wait는 run의 `execCtx`(= `OnCancel`이 호출하는 cancel과 연결)로 대기하므로 큐에서 대기 중인 run도 취소 시 큐에서 빠지고 reject로 종결된다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- predecessor `03_queue_capability_contract` 완료 근거가 있는지 확인한다. +- queue admission이 FIFO이며 goroutine/context leak 가능성이 없는지 확인한다. +- foreground/background semantics와 store/event side effects가 테스트와 일치하는지 확인한다. +- full-cycle/e2e 생략이 있으면 사유와 남은 위험이 기록됐는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### REFACTOR-1 중간 검증 +```bash +$ go test -count=1 ./apps/node/internal/node -run 'Test.*Queue|TestConcurrency' +ok iop/apps/node/internal/node 0.444s +``` +포함 테스트(모두 PASS): TestAdmissionQueue_ForegroundSecondWaitsThenRuns, _BackgroundQueuedReturnsThenRuns, _FIFOOrdering, _MaxQueueOverflow, _QueueTimeout, _ReleaseAfterFailurePromotesNext, _ReleaseAfterCancelPromotesNext, _PermitReleasedAfterCompletion, _AdapterCapQueues, _GlobalBeatsAdapterCap, _GlobalBeatsAdapterCap_DifferentAdapters, TestConcurrencyLimit_Unlimited, _RejectStoreAndEvent, TestQueueOverflow_RejectEventObservedByEdge. + +### REFACTOR-2 중간 검증 +```bash +$ go test -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 0.459s +``` + +### REFACTOR-3 중간 검증 +```bash +$ go test -count=1 ./apps/node/internal/node -run 'Test.*Reject|Test.*Queue|TestConcurrencyLimit_.*Event' +ok iop/apps/node/internal/node 0.436s +``` + +### 추가 검증 (race detector) +```bash +$ go test -race -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 1.691s +``` + +### 최종 검증 +```bash +$ go test -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 0.459s + +$ go test -count=1 ./apps/node/... +ok iop/apps/node/cmd/node 0.014s +ok iop/apps/node/internal/adapters 0.016s +ok iop/apps/node/internal/adapters/cli 46.772s +? iop/apps/node/internal/adapters/cli/internal/testutil [no test files] +ok iop/apps/node/internal/adapters/cli/status 39.730s +? iop/apps/node/internal/adapters/mock [no test files] +ok iop/apps/node/internal/adapters/ollama 0.016s +ok iop/apps/node/internal/adapters/vllm 0.015s +ok iop/apps/node/internal/bootstrap 0.267s +ok iop/apps/node/internal/node 0.459s +ok iop/apps/node/internal/router 0.007s +? iop/apps/node/internal/runtime [no test files] +ok iop/apps/node/internal/store 0.058s +ok iop/apps/node/internal/terminal 0.576s +ok iop/apps/node/internal/transport 5.141s + +$ ./scripts/e2e-smoke.sh +[e2e] Auxiliary smoke test PASSED. +[e2e] Completion still requires scripts/dev/edge.sh + scripts/dev/node.sh user-flow verification. +``` +(보조 smoke: mock/fake-cli adapter로 같은 session 메시지 3회가 start → `[node-message]` → complete 순서로 정상 통과. 새 admission path가 기존 정상 실행 흐름을 회귀시키지 않음을 확인.) + +### full-cycle 수동 검증 생략 사유 / 남은 위험 +- node-smoke 필수 검증 중 "repo 내부 edge-node 진단 + full-cycle 실제 구동"(`scripts/dev/edge.sh` + `scripts/dev/node.sh` 사용자 흐름, 외부 Edge runtime `toki-labs.com:19090`)은 현재 작업 컨테이너에서 외부 field host/TCP 접근이 불가하여 생략했다(node-smoke 차단 기준: field host 또는 Edge-Node TCP port 접근 불가). +- 대체로 보조 `./scripts/e2e-smoke.sh`(mock adapter full pipeline)와 deterministic unit/integration + `-race`로 admission/queue/release/reject 경로를 커버했다. +- 남은 위험: 실제 외부 provider(ollama/vllm/CLI)가 `MaxQueue`/`QueueTimeoutMS`를 capabilities로 채워 보내는 실환경에서의 큐 동작은 mock 기준으로만 검증됨. 실 Edge 연동 full-cycle 검증은 원격 runner에서 별도 수행 필요. + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** + +## 코드리뷰 결과 + +- 종합 판정: FAIL +- 차원별 평가: + - correctness: Fail + - completeness: Fail + - test coverage: Fail + - API contract: Pass + - code quality: Pass + - plan deviation: Fail + - verification trust: Fail +- 발견된 문제: + - Required: [apps/node/internal/node/run_manager.go](/config/workspace/iop/apps/node/internal/node/run_manager.go:278)에서 `enqueue()`가 adapter gate가 아직 승격되지 않은 요청까지 global gate에 즉시 `reserve()`합니다. 이 상태에서 global slot이 비면 adapter slot을 아직 얻지 못한 요청이 global head를 선점할 수 있어, 다른 adapter의 큐 head가 실행 가능한데도 global slot을 쓰지 못합니다. 예: global=2, adapter A/B capacity=1에서 A1/B1 실행 중 A2를 먼저 큐에 넣고 B2를 큐에 넣은 뒤 B1만 release하면, B2는 adapter B slot이 비었는데도 A2가 global slot을 선점해 A1 release 전까지 시작하지 못합니다. `max_queue`/`queue_timeout`도 이 숨은 global 대기에 일관되게 적용되지 않습니다. 수정은 adapter gate가 실제로 grant된 뒤 global gate를 reserve/wait하도록 합성 순서를 바꾸고, 위 교차 adapter 회귀 테스트를 추가하세요. + - Required: [code_review_cloud_G06_0.log](/config/workspace/iop/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log:155)에 필수 `scripts/dev/edge.sh` + `scripts/dev/node.sh` repo 내부 edge-node 진단/full-cycle 검증이 생략된 상태로 남아 있습니다. 이 작업은 node 실행/admission 경로 변경이므로 `agent-test/local/node-smoke.md` 기준상 보조 `./scripts/e2e-smoke.sh`만으로 완료 증거가 되지 않습니다. 후속 작업에서 임시 config/포트로 해당 진단을 실제 수행하거나, 수행 불가 시 실행한 명령과 실제 stdout/stderr가 있는 blocker evidence를 남기세요. +- 다음 단계: WARN/FAIL 후속 plan/review 파일을 작성한다. diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_1.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_1.log new file mode 100644 index 0000000..22707c7 --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_1.log @@ -0,0 +1,256 @@ + + +# Code Review Reference - REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Evidence gaps that a follow-up agent can close by rerunning commands or collecting artifacts are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record the needed decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-14 +task=m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue, plan=1, tag=REVIEW_REFACTOR + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Task ids: + - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. + - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. + - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G06.md` -> `code_review_cloud_G06_N.log`, `PLAN-cloud-G06.md` -> `plan_cloud_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_REFACTOR-1] Gate Composition Fix | [x] | +| [REVIEW_REFACTOR-2] Verification Evidence Recovery | [x] | + +## 구현 체크리스트 + +- [x] 교차 adapter global head-of-line blocking을 재현하는 failing regression test를 추가한다. (`TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine`: global=2, adapter A/B cap=1. A1/B1 실행 중 A2→B2 enqueue 후 B1만 release하면 B2가 즉시 시작, A2는 a1 release 전까지 queued 유지. 수정 전 `enqueue`에서는 A2가 freed global slot을 선점해 이 테스트가 실패한다.) +- [x] adapter gate가 실제 grant된 뒤에만 global gate를 reserve/wait하도록 admission ticket 흐름을 수정한다. (`enqueue()`는 adapter gate만 reserve. `wait()`에서 adapter grant 후 global을 reserve/wait.) +- [x] global wait 경로에서도 provider queue timeout 또는 admission timeout 정책이 일관되게 적용되는지 테스트로 고정한다. (`wait()`가 adapter `queueTimeout`로 단일 deadline context를 파생해 adapter+global wait 전체에 적용. `mapDeadline`이 deadline 기인 취소를 `queue_timeout` reason으로 재분류. `TestAdmissionQueue_QueueTimeoutAppliesAcrossGates`로 global-wait 단계 timeout 고정.) +- [x] 기존 FIFO ordering, max_queue overflow, queue_timeout, success/failure/cancel release tests가 계속 통과하게 한다. (전부 PASS.) +- [x] 필수 repo 내부 edge-node 진단 또는 full-cycle 검증 증거를 `agent-ops/skills/project/e2e-smoke/SKILL.md` 기준으로 기록한다. (아래 `repo 내부 edge-node 진단 / full-cycle 증거` 참조.) +- [x] 최종 검증 명령을 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G06_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G06_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리 `agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`를 `agent-task/archive/YYYY/MM/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-provider-availability-capacity-queue-foundation/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-cloud-G06.md`와 `CODE_REVIEW-cloud-G06.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- **`fifoGate.waitOn`에서 내부 timer 제거 → ctx deadline 일원화.** 계획은 global wait 경로 timeout 적용만 요구했으나, gate별 개별 timer를 두면 adapter wait와 global wait에 timeout이 두 번 적용되어 총 대기가 최대 2×queueTimeout이 될 수 있었다. 이를 막기 위해 `waitOn`의 `applyTimeout` 파라미터와 내부 `time.Timer`를 제거하고, `admissionTicket.wait()`가 adapter `queueTimeout`로 단일 `context.WithTimeout`을 파생해 adapter+global 두 단계 전체에 하나의 deadline만 적용하도록 했다. deadline 기인 취소는 `mapDeadline`이 `queue_timeout` reason으로 재분류한다. +- 그 외에는 계획의 작업 항목(enqueue=adapter-only, wait=adapter→global, 실패 시 adapter slot release, 교차 adapter 회귀 테스트, global-wait timeout 테스트)을 그대로 따랐다. + +## 주요 설계 결정 + +- **gate 합성 순서 = enqueue(adapter only) → wait(adapter grant 후 global reserve+wait).** 1차 구현의 결함(enqueue 시점에 adapter 대기 ticket이 global slot까지 선점 → 교차 adapter head-of-line blocking)을 근본 제거했다. global slot은 자기 adapter slot을 이미 보유한 run만 점유하므로, 한 provider의 큐 대기가 다른 provider의 실행 가능한 큐 head를 막지 못한다. +- **단일 deadline로 admission 대기 bound.** adapter `queueTimeout`만을 권위값으로 사용(global gate는 무제한/무timeout). adapter 또는 global wait 어디서 막혀도 동일 정책으로 종료되고 무기한 대기가 없다. +- **`max_queue` 우회 방지.** overflow 판정은 `enqueue()`의 adapter `reserve()` 단계에서만 일어나고, global gate는 무제한 큐라 max_queue를 우회하지 않는다. +- **release/leak/cancel 안전성 유지.** `wait()`는 실패 시 보유한 adapter slot을 `ticket.release()`로 항상 unwind. run의 `defer ticket.release()`가 success/failure/cancel/timeout 모든 종료 경로에서 슬롯 반납. cancel-while-queued는 run `execCtx`로 wait를 취소해 큐에서 제거. race detector clean(`go test -race`). + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- global gate가 adapter gate grant 전에 선점되지 않는지 확인한다. +- 교차 adapter 회귀 테스트가 실패 조건을 실제로 고정하는지 확인한다. +- queue timeout 또는 admission timeout 정책이 global wait 경로에서도 무기한 대기를 만들지 않는지 확인한다. +- repo 내부 edge-node 진단/full-cycle 증거가 보조 smoke와 구분되어 기록됐는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. +- mobile/UI hang, timeout, 또는 2분 무진행은 blind retry를 중단하고 focused rerun 명령과 screenshot/window/UI-tree evidence path를 남기며, 불가능하면 정확한 사유를 남긴다. + +### REVIEW_REFACTOR-1 중간 검증 +```bash +$ go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Global|TestAdmissionQueue_.*Queue|TestConcurrencyLimit' +--- PASS: TestAdmissionQueue_BackgroundQueuedReturnsThenRuns (0.01s) +--- PASS: TestAdmissionQueue_MaxQueueOverflow (0.00s) +--- PASS: TestAdmissionQueue_QueueTimeout (0.12s) +--- PASS: TestConcurrencyLimit_Unlimited (0.00s) +--- PASS: TestAdmissionQueue_AdapterCapQueues (0.01s) +--- PASS: TestAdmissionQueue_GlobalBeatsAdapterCap (0.01s) +--- PASS: TestAdmissionQueue_GlobalBeatsAdapterCap_DifferentAdapters (0.01s) +--- PASS: TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine (0.11s) +--- PASS: TestAdmissionQueue_QueueTimeoutAppliesAcrossGates (0.12s) +--- PASS: TestConcurrencyLimit_RejectStoreAndEvent (0.00s) +PASS +ok iop/apps/node/internal/node 0.387s +``` +신규/핵심: `TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine`(교차 adapter 회귀), `TestAdmissionQueue_QueueTimeoutAppliesAcrossGates`(global-wait 단계 timeout). + +### REVIEW_REFACTOR-2 중간 검증 +```bash +$ ./scripts/e2e-smoke.sh +[e2e] Auxiliary smoke test PASSED. +[e2e] Completion still requires scripts/dev/edge.sh + scripts/dev/node.sh user-flow verification. +``` +(보조 smoke. full-cycle 증거는 아래 `repo 내부 edge-node 진단 / full-cycle 증거` 참조.) + +### 최종 검증 +```bash +$ go test -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 0.672s + +$ go test -race -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 1.924s + +$ go test -count=1 ./apps/node/... +ok iop/apps/node/cmd/node 0.012s +ok iop/apps/node/internal/adapters 0.010s +ok iop/apps/node/internal/adapters/cli 47.077s +? iop/apps/node/internal/adapters/cli/internal/testutil [no test files] +ok iop/apps/node/internal/adapters/cli/status 39.853s +? iop/apps/node/internal/adapters/mock [no test files] +ok iop/apps/node/internal/adapters/ollama 0.012s +ok iop/apps/node/internal/adapters/vllm 0.013s +ok iop/apps/node/internal/bootstrap 0.265s +ok iop/apps/node/internal/node 0.675s +ok iop/apps/node/internal/router 0.005s +? iop/apps/node/internal/runtime [no test files] +ok iop/apps/node/internal/store 0.043s +ok iop/apps/node/internal/terminal 0.563s +ok iop/apps/node/internal/transport 5.137s + +$ ./scripts/e2e-smoke.sh +[e2e] Auxiliary smoke test PASSED. +``` + +### repo 내부 edge-node 진단 / full-cycle 증거 + +`agent-ops/skills/project/e2e-smoke/SKILL.md` 기준 repo 내부 edge-node 진단을 실제로 수행했다. 1차 log의 "외부 field host 필요" 판단은 오인이었고, `scripts/dev/edge.sh`(`edge console`)와 `scripts/dev/node.sh`(`node serve`)는 임시 config/포트로 **로컬 두 프로세스**로 구동된다. 본 진단은 두 스크립트를 deterministic `fake-cli` adapter로 구동해 실행했다(driver: `IOP_E2E_PROMPT_INDEX=1 ./scripts/e2e-smoke.sh`가 `scripts/dev/edge.sh` < fifo, `scripts/dev/node.sh`를 별도 프로세스로 띄움; 본문 라인 247/261). + +```text +검증 결과 +- Go 테스트: go test ./apps/node/... - 통과 +- Repo 내부 edge-node 진단: 통과 (scripts/dev/edge.sh + scripts/dev/node.sh, 임시 edge.yaml/node.yaml, port=37256, fake-cli) +- 메시지 왕복: message x2 (foreground, session=default) - 통과 +- Node message relay: node local payload == edge rendered payload (run별 내용/순서 동일) - 통과 (check_node_messages_relayed_to_edge diff 통과) +- Edge command 응답: /nodes, /capabilities, /transport, /sessions, /terminate-session - 통과 +- Background run: /background on → session2 background run start/msg/complete - 통과 +- 보조 E2E smoke: mock - 통과 (위 진단과 동일 실행에서 수행) +- Full-cycle 실제 구동: 통과 (register → /nodes → /capabilities → /transport → fg msg x2 → bg run → /sessions → /terminate-session) +- Real CLI profile: 미실행 (실제 claude/codex/antigravity/opencode CLI/계정 미구비; 본 변경은 admission/queue 로직으로 deterministic fake-cli로 충분히 커버, 실 provider 큐 동작은 원격 runner 별도 검증 권장) +- 남은 위험: 실 provider가 capabilities로 MaxQueue/QueueTimeoutMS를 채워 보내는 환경의 큐 동작은 mock 기준 검증. priority/weighted queue는 범위 외. +``` + +핵심 edge console 출력(요약, port=37256): +```text +edge> [node0-evt] connected reason="registered" + node0 = test-node (test-node) +edge> [node0-capabilities] adapter=cli target=fake-cli session=default + capacity = 4 / in_flight = 0 / queued = 0 / targets = fake-cli +edge> [node0-transport] connected = true / state = connected / node_id = test-node +edge> [edge] sent run_id=manual-...771988542 ... background=false +[node0-evt] start run_id=manual-...771988542 +[node0-msg] IOP_E2E_HELLO_BASIC +[node0-msg] IOP_E2E_HELLO_BASIC_TAIL +[node0-evt] complete run_id=manual-...771988542 detail="idle-timeout" +edge> [edge] sent run_id=manual-...008341167 ... background=false +[node0-evt] start run_id=manual-...008341167 +[node0-msg] IOP_E2E_HELLO_FORMAL +[node0-msg] IOP_E2E_HELLO_FORMAL_TAIL +[node0-evt] complete run_id=manual-...008341167 detail="idle-timeout" +edge> session → session2 ; background → on +edge> [node0-evt] start run_id=manual-...539497543 session=session2 background=true +[node0-msg] IOP_E2E_THANKS_SHORT +[node0-msg] IOP_E2E_THANKS_SHORT_TAIL +[node0-evt] complete run_id=manual-...539497543 detail="idle-timeout" +edge> [node0-sessions] sessions: 2 + [0] mode=persistent target=fake-cli session=default + [1] mode=persistent target=fake-cli session=session2 +edge> terminated session session2 node=node0 +``` +node-local `[node-message]` payload 목록(run별)과 edge `[node0-msg]` 목록이 내용/순서까지 동일하고, 각 run의 complete가 마지막 message 이후에만 도착했다(skill 판정 기준 충족). + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +Sections and their ownership: + +| 섹션 | 소유자 | 설명 | +|------|--------|------| +| 헤더 주석, 개요(date/task/plan/tag), 리뷰 에이전트 지시 | 스텁 생성 시 고정 | 구현 에이전트가 수정하거나 실행하지 않음 | +| Roadmap Targets | follow-up plan에서 복사해 스텁 생성 시 고정 | 구현 에이전트가 수정하지 않음; PASS 시 code-review가 `complete.log`의 `Roadmap Completion`으로 복사 | +| 구현 항목별 완료 여부 (항목명) | 스텁 생성 시 고정 | `[ ]` -> `[x]` 체크만 구현 에이전트가 수행 | +| 구현 체크리스트 (항목 텍스트/순서) | follow-up plan에서 복사해 스텁 생성 시 고정 | 구현 에이전트가 `[ ]` -> `[x]` 체크만 수행; 마지막 체크박스는 저장 전 필수 | +| 코드리뷰 전용 체크리스트 | Review agent only | Implementing agent must not modify or check this section | +| 계획 대비 변경 사항, 주요 설계 결정 | 구현 에이전트가 채움 | placeholder 텍스트를 실제 내용으로 교체 | +| 사용자 리뷰 요청 | 구현 에이전트가 채움 | 진행에 사용자 입력이 필요하지 않으면 `상태: 없음` 유지; 구현 중 직접 질문은 금지; 필요하면 결정 항목, 근거, 명령 출력, 자동 후속 불가 이유, 재개 조건을 기록 | +| 리뷰어를 위한 체크포인트 | 스텁 생성 시 고정 | 계획에서 추출한 리뷰 포인트 | +| 검증 결과 (섹션 제목 + 명령) | 스텁 생성 시 고정 | 실행 출력만 구현 에이전트가 채움; 명령 변경은 `계획 대비 변경 사항`에 기록 | +| 코드리뷰 결과 | 리뷰 에이전트가 append | 스텁에 포함하지 않음 | + +## 코드리뷰 결과 + +- 종합 판정: FAIL +- 차원별 평가: + - correctness: Fail + - completeness: Fail + - test coverage: Fail + - API contract: Pass + - code quality: Pass + - plan deviation: Pass + - verification trust: Pass +- 발견된 문제: + - Required: [run_manager.go](/config/workspace/iop/apps/node/internal/node/run_manager.go:332)의 `mapDeadline`은 `ctx.Err() == context.DeadlineExceeded`이면 어떤 deadline인지 구분하지 않고 `queue_timeout`으로 재분류합니다. 그런데 `ticket.wait(execCtx)`에 전달되는 `execCtx`는 [node.go](/config/workspace/iop/apps/node/internal/node/node.go:141)에서 `RunRequest.timeout_sec`로도 deadline이 걸릴 수 있습니다. 따라서 `queue_timeout_ms=0`인 provider queue에서 queued run이 request timeout으로 취소되어도 `queue timeout exceeded (queue_timeout=0s)`로 store/event에 기록되고, `queue_timeout_ms > timeout_sec`인 경우도 실제 원인은 request timeout인데 queue timeout으로 오분류됩니다. `queue_timeout`은 queue timeout mechanism이 실제로 만료된 경우에만 반환하도록 timeout cause/channel을 분리하고, parent deadline/cancel은 `concurrency_unavailable` 또는 별도 request-timeout reason으로 유지하는 회귀 테스트를 추가하세요. +- 다음 단계: WARN/FAIL 후속 plan/review 파일을 작성한다. diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log new file mode 100644 index 0000000..0d1b0d8 --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log @@ -0,0 +1,202 @@ + + +# Code Review Reference - REVIEW_REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Evidence gaps that a follow-up agent can close by rerunning commands or collecting artifacts are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record the needed decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-14 +task=m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue, plan=2, tag=REVIEW_REVIEW_REFACTOR + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Task ids: + - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. + - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. + - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G06.md` -> `code_review_cloud_G06_N.log`, `PLAN-cloud-G06.md` -> `plan_cloud_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_REVIEW_REFACTOR-1] Timeout Cause Classification | [x] | + +## 구현 체크리스트 + +- [x] `RunRequest.timeout_sec` 등 parent deadline이 queue timeout으로 오분류되지 않는 회귀 테스트를 추가한다. (`TestAdmissionQueue_ParentTimeoutNotQueueTimeout`: queue_timeout_ms=0에서 TimeoutSec:1 queued run이 parent timeout으로 종료 — error/store에 "queue timeout"/"queue_timeout=0s" 없음. `TestAdmissionQueue_ParentTimeoutBeatsQueueTimeout`: queue_timeout_ms=10s인데 TimeoutSec=1로 parent deadline이 먼저 와도 queue timeout 오분류 안 함.) +- [x] provider `queue_timeout_ms` deadline이 실제로 만료된 경우에만 `queue_timeout` reason을 반환하도록 admission wait timeout cause를 분리한다. (`context.WithTimeoutCause(ctx, queueTimeout, errAdmissionQueueTimeout)` + `mapDeadline`이 `context.Cause(ctx) == errAdmissionQueueTimeout`일 때만 `queue_timeout` 반환.) +- [x] `queue_timeout_ms=0`인 queued request가 parent deadline으로 취소될 때 store/event error에 `queue_timeout=0s`가 남지 않게 한다. (queueTimeout<=0이면 timeout context 자체를 파생하지 않으므로 cause가 절대 `errAdmissionQueueTimeout`이 아니고, reason은 `concurrency_unavailable` 유지.) +- [x] 기존 queue timeout, global-wait timeout, cross-adapter head-of-line tests가 계속 통과하게 한다. (`TestAdmissionQueue_QueueTimeout`, `_QueueTimeoutAppliesAcrossGates`는 여전히 실제 provider queue timeout에서 `queue timeout` reason 확인; `_GlobalNoCrossAdapterHeadOfLine` 통과.) +- [x] 최종 검증 명령을 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G06_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G06_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [x] PASS이면 active task 디렉터리 `agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`를 `agent-task/archive/YYYY/MM/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-provider-availability-capacity-queue-foundation/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-cloud-G06.md`와 `CODE_REVIEW-cloud-G06.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- 계획의 권장 방식 중 첫 번째 후보(`context.WithTimeoutCause` + `context.Cause`)를 그대로 채택했다. 공유 `time.Timer`/channel 방식은 채택하지 않았다 — cause 기반이 더 적은 코드로 parent/queue deadline을 정확히 구분하고 기존 `waitOn` 단일-select 구조를 유지하기 때문이다. 그 외 계획과의 차이 없음. + +## 주요 설계 결정 + +- **deadline cause 분리.** `admissionTicket.wait()`가 provider `queueTimeout`로 파생하는 timeout context를 `context.WithTimeoutCause(ctx, queueTimeout, errAdmissionQueueTimeout)`로 만들고, `mapDeadline`은 `errors.Is(context.Cause(ctx), errAdmissionQueueTimeout)`일 때만 `queue_timeout` reason을 반환한다. parent ctx 취소나 parent deadline(`RunRequest.timeout_sec`)이 원인이면 `context.Cause`가 `errAdmissionQueueTimeout`이 아니므로 `concurrency_unavailable`을 유지한다. +- **`queue_timeout_ms=0` = 메커니즘 비활성.** queueTimeout<=0이면 timeout context를 아예 파생하지 않으므로 `queue_timeout=0s` 같은 잘못된 메시지가 store/event에 남을 수 없다. +- 기존 `queue_timeout` 의미는 보존: 실제 provider queue timeout 만료에서만 `queue_timeout` reason이 나오며, 기존 두 timeout 테스트가 이를 계속 검증한다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- parent request timeout과 provider queue timeout이 서로 다른 cause로 분류되는지 확인한다. +- `queue_timeout_ms=0`에서 `queue_timeout=0s`가 store/event에 남지 않는지 확인한다. +- 기존 queue timeout 테스트는 여전히 실제 provider queue timeout을 검증하는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. +- mobile/UI hang, timeout, 또는 2분 무진행은 blind retry를 중단하고 focused rerun 명령과 screenshot/window/UI-tree evidence path를 남기며, 불가능하면 정확한 사유를 남긴다. + +### REVIEW_REVIEW_REFACTOR-1 중간 검증 +```bash +$ go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Timeout|TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine' +--- PASS: TestAdmissionQueue_QueueTimeout (0.12s) +--- PASS: TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine (0.11s) +--- PASS: TestAdmissionQueue_QueueTimeoutAppliesAcrossGates (0.12s) +--- PASS: TestAdmissionQueue_ParentTimeoutNotQueueTimeout (1.00s) +--- PASS: TestAdmissionQueue_ParentTimeoutBeatsQueueTimeout (1.00s) +ok iop/apps/node/internal/node 2.363s +``` +- `TestAdmissionQueue_QueueTimeout`, `_QueueTimeoutAppliesAcrossGates`: 실제 provider queue timeout → `queue timeout` reason 유지(회귀 없음). +- `TestAdmissionQueue_ParentTimeoutNotQueueTimeout`(queue_timeout_ms=0), `_ParentTimeoutBeatsQueueTimeout`(queue_timeout_ms=10s > TimeoutSec=1): parent request timeout → `queue timeout`/`queue_timeout=0s` 미포함. + +### 최종 검증 +```bash +$ go test -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 2.679s + +$ go test -race -count=1 ./apps/node/internal/node +ok iop/apps/node/internal/node 3.954s + +$ go test -count=1 ./apps/node/... +ok iop/apps/node/cmd/node 0.014s +ok iop/apps/node/internal/adapters 0.015s +ok iop/apps/node/internal/adapters/cli 46.757s +? iop/apps/node/internal/adapters/cli/internal/testutil [no test files] +ok iop/apps/node/internal/adapters/cli/status 39.775s +? iop/apps/node/internal/adapters/mock [no test files] +ok iop/apps/node/internal/adapters/ollama 0.016s +ok iop/apps/node/internal/adapters/vllm 0.015s +ok iop/apps/node/internal/bootstrap 0.265s +ok iop/apps/node/internal/node 2.684s +ok iop/apps/node/internal/router 0.008s +? iop/apps/node/internal/runtime [no test files] +ok iop/apps/node/internal/store 0.066s +ok iop/apps/node/internal/terminal 0.564s +ok iop/apps/node/internal/transport 5.151s + +$ ./scripts/e2e-smoke.sh +[e2e] Auxiliary smoke test PASSED. +[e2e] Completion still requires scripts/dev/edge.sh + scripts/dev/node.sh user-flow verification. +``` +(이번 후속은 admission timeout 분류만 변경. repo 내부 edge-node 진단 full-cycle 증거는 `code_review_cloud_G06_1.log`에서 회복되었고, 위 smoke로 admission 변경이 정상 실행 흐름을 회귀시키지 않음을 재확인했다.) + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +Sections and their ownership: + +| 섹션 | 소유자 | 설명 | +|------|--------|------| +| 헤더 주석, 개요(date/task/plan/tag), 리뷰 에이전트 지시 | 스텁 생성 시 고정 | 구현 에이전트가 수정하거나 실행하지 않음 | +| Roadmap Targets | follow-up plan에서 복사해 스텁 생성 시 고정 | 구현 에이전트가 수정하지 않음; PASS 시 code-review가 `complete.log`의 `Roadmap Completion`으로 복사 | +| 구현 항목별 완료 여부 (항목명) | 스텁 생성 시 고정 | `[ ]` -> `[x]` 체크만 구현 에이전트가 수행 | +| 구현 체크리스트 (항목 텍스트/순서) | follow-up plan에서 복사해 스텁 생성 시 고정 | 구현 에이전트가 `[ ]` -> `[x]` 체크만 수행; 마지막 체크박스는 저장 전 필수 | +| 코드리뷰 전용 체크리스트 | Review agent only | Implementing agent must not modify or check this section | +| 계획 대비 변경 사항, 주요 설계 결정 | 구현 에이전트가 채움 | placeholder 텍스트를 실제 내용으로 교체 | +| 사용자 리뷰 요청 | 구현 에이전트가 채움 | 진행에 사용자 입력이 필요하지 않으면 `상태: 없음` 유지; 구현 중 직접 질문은 금지; 필요하면 결정 항목, 근거, 명령 출력, 자동 후속 불가 이유, 재개 조건을 기록 | +| 리뷰어를 위한 체크포인트 | 스텁 생성 시 고정 | 계획에서 추출한 리뷰 포인트 | +| 검증 결과 (섹션 제목 + 명령) | 스텁 생성 시 고정 | 실행 출력만 구현 에이전트가 채움; 명령 변경은 `계획 대비 변경 사항`에 기록 | +| 코드리뷰 결과 | 리뷰 에이전트가 append | 스텁에 포함하지 않음 | + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - correctness: Pass + - completeness: Pass + - test coverage: Pass + - API contract: Pass + - code quality: Pass + - plan deviation: Pass + - verification trust: Pass +- 발견된 문제: + - Required: 없음 + - Suggested: 없음 + - Nit: [run_manager.go](/config/workspace/iop/apps/node/internal/node/run_manager.go:256)의 `admissionTicket`에 현재 읽히지 않는 bookkeeping 필드(`globalCh`, `queued`)가 남아 있습니다. 동작에는 영향이 없고 다음 snapshot/observability 작업과 충돌하지 않도록 별도 follow-up을 요구하지 않습니다. +- 리뷰어 검증: + - `go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Timeout|TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine'` - PASS (`ok iop/apps/node/internal/node 2.367s`) + - `go test -count=1 ./apps/node/internal/node` - PASS (`ok iop/apps/node/internal/node 2.676s`) + - `go test -race -count=1 ./apps/node/internal/node` - PASS (`ok iop/apps/node/internal/node 3.973s`) + - `go test -count=1 ./apps/node/...` - PASS + - `./scripts/e2e-smoke.sh` - PASS (`[e2e] Auxiliary smoke test PASSED.`) + - `git diff --check` - PASS +- 다음 단계: PASS 종결. active plan/review를 log로 아카이브하고 `complete.log` 작성 후 task 디렉터리를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/complete.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/complete.log new file mode 100644 index 0000000..b9f9986 --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/complete.log @@ -0,0 +1,54 @@ +# Complete - m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue + +## 완료 일시 + +2026-06-14 + +## 요약 + +Provider FIFO admission queue foundation을 3개 리뷰 루프 끝에 PASS 종결했다. 최종 루프에서 parent request timeout과 provider queue timeout cause 분리를 확인했고, queue admission/release/reject 경로의 deterministic test, race, node 패키지 전체, smoke 검증을 통과했다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G06_0.log` | `code_review_cloud_G06_0.log` | FAIL | gate composition이 adapter 대기 요청에 global slot을 선점시킬 수 있었고, repo 내부 edge-node full-cycle 증거가 부족했다. | +| `plan_cloud_G06_1.log` | `code_review_cloud_G06_1.log` | FAIL | parent `RunRequest.timeout_sec` deadline이 provider `queue_timeout`으로 오분류될 수 있었다. | +| `plan_cloud_G06_2.log` | `code_review_cloud_G06_2.log` | PASS | queue timeout cause를 `context.WithTimeoutCause`로 분리했고, parent deadline 회귀 테스트와 전체 검증을 통과했다. | + +## 구현/정리 내용 + +- `permitManager` 기반 reject-on-exceed를 provider별 FIFO `fifoGate`/`admissionManager`/`admissionTicket` 흐름으로 교체했다. +- `OnRunRequest`가 요청을 `queued`로 기록한 뒤 capacity가 있으면 `running`으로 승격하고, capacity가 차면 provider FIFO queue에서 대기하도록 변경했다. +- 실행 완료, 실패, cancel, timeout 경로에서 ticket release가 queue head를 승격하도록 정리했다. +- `queue_full`, `queue_timeout`, `concurrency_unavailable` admission reason을 store error와 RunEvent error에 남기도록 했다. +- 교차 adapter global head-of-line blocking을 막기 위해 adapter slot이 grant된 뒤에만 global gate를 reserve/wait하도록 수정했다. +- provider queue timeout 전용 cause를 `context.WithTimeoutCause`로 분리해 parent request timeout이 `queue_timeout`으로 오분류되지 않게 했다. +- FIFO ordering, queue overflow, queue timeout, global wait timeout, parent deadline 회귀, release-on-failure/cancel/completion 테스트를 추가/갱신했다. + +## 최종 검증 + +- `go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Timeout|TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine'` - PASS; `ok iop/apps/node/internal/node 2.367s` +- `go test -count=1 ./apps/node/internal/node` - PASS; `ok iop/apps/node/internal/node 2.676s` +- `go test -race -count=1 ./apps/node/internal/node` - PASS; `ok iop/apps/node/internal/node 3.973s` +- `go test -count=1 ./apps/node/...` - PASS; node 하위 패키지 전체 통과 +- `./scripts/e2e-smoke.sh` - PASS; `[e2e] Auxiliary smoke test PASSED.` +- `git diff --check` - PASS +- `code_review_cloud_G06_1.log`의 repo 내부 edge-node 진단/full-cycle 증거 - PASS; `scripts/dev/edge.sh` + `scripts/dev/node.sh` 로컬 두 프로세스, fake-cli foreground/background/session command 흐름 통과 + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Completed task ids: + - `admission-gate`: PASS; evidence=`agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log`, `agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log`; verification=`go test -count=1 ./apps/node/internal/node`, `go test -race -count=1 ./apps/node/internal/node`, `go test -count=1 ./apps/node/...`, `./scripts/e2e-smoke.sh` + - `queue-release`: PASS; evidence=`agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log`, `agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log`; verification=`go test -count=1 ./apps/node/internal/node`, `go test -race -count=1 ./apps/node/internal/node`, `go test -count=1 ./apps/node/...`, `./scripts/e2e-smoke.sh` + - `queue-reject`: PASS; evidence=`agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log`, `agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_2.log`; verification=`go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Timeout|TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine'`, `go test -count=1 ./apps/node/internal/node`, `go test -race -count=1 ./apps/node/internal/node`, `go test -count=1 ./apps/node/...`, `./scripts/e2e-smoke.sh` +- Not completed task ids: 없음 + +## 잔여 Nit + +- `apps/node/internal/node/run_manager.go`의 `admissionTicket.globalCh`/`queued`는 현재 읽히지 않는 private bookkeeping 필드다. 동작 영향은 없으며 후속 snapshot/observability 작업과 함께 정리 가능하다. + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/PLAN-cloud-G06.md b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_0.log similarity index 100% rename from agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/PLAN-cloud-G06.md rename to agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_0.log diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_1.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_1.log new file mode 100644 index 0000000..80e5ab5 --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_1.log @@ -0,0 +1,107 @@ + + +# Plan - FIFO Admission Queue Follow-up + +## 이 파일을 읽는 구현 에이전트에게 + +이 계획은 `code_review_cloud_G06_0.log`의 Required 문제만 해결한다. 구현의 마지막 단계는 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것이다. 검증을 실행하고 실제 stdout/stderr를 붙여 넣은 뒤 active 파일을 그대로 둔 채 리뷰 준비 완료를 보고한다. 사용자 결정, 사용자 소유 외부 환경, scope conflict 없이는 사용자에게 직접 묻지 말고, 차단 시 review stub의 `사용자 리뷰 요청` 섹션에 근거를 남긴다. `USER_REVIEW.md`, `complete.log`, log/archive 이동은 code-review 전용이다. + +## 배경 + +1차 구현은 provider별 FIFO queue, queue overflow/timeout, store/event reason을 대부분 구현했지만, adapter gate와 global gate를 enqueue 시점에 동시에 reserve하면서 교차 adapter head-of-line blocking이 생길 수 있다. 또한 필수 repo 내부 edge-node 진단/full-cycle 증거가 보조 smoke로 대체되어 verification trust가 부족하다. + +## 사용자 리뷰 요청 흐름 + +구현 중 사용자-only blocker는 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 에이전트는 직접 질문, 채팅 선택지 제시, `request_user_input` 호출을 하지 않는다. code-review가 요청 타당성을 검증하고 실제 `USER_REVIEW.md` 작성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Task ids: + - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. + - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. + - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. +- Completion mode: check-on-pass + +## 범위 결정 근거 + +- 포함: `admissionManager`/`admissionTicket`의 adapter/global gate 합성 순서 수정, global bottleneck 회귀 테스트, 필수 node smoke/full-cycle 증거 회복. +- 제외: `ProviderSnapshot.Queued` 채우기, Edge/Control Plane 변경, priority/weighted/preemptive queue. +- 1차 구현의 정상 FIFO, overflow, timeout, cancel/release tests는 유지하고 필요한 경우만 조정한다. + +## 구현 체크리스트 + +- [ ] 교차 adapter global head-of-line blocking을 재현하는 failing regression test를 추가한다. +- [ ] adapter gate가 실제 grant된 뒤에만 global gate를 reserve/wait하도록 admission ticket 흐름을 수정한다. +- [ ] global wait 경로에서도 provider queue timeout 또는 admission timeout 정책이 일관되게 적용되는지 테스트로 고정한다. +- [ ] 기존 FIFO ordering, max_queue overflow, queue_timeout, success/failure/cancel release tests가 계속 통과하게 한다. +- [ ] 필수 repo 내부 edge-node 진단 또는 full-cycle 검증 증거를 `agent-ops/skills/project/e2e-smoke/SKILL.md` 기준으로 기록한다. +- [ ] 최종 검증 명령을 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 작업 항목 + +### [REVIEW_REFACTOR-1] Gate Composition Fix + +문제: [run_manager.go](/config/workspace/iop/apps/node/internal/node/run_manager.go:278)의 `enqueue()`는 adapter gate에서 대기 중인 ticket도 global gate에 즉시 등록한다. 이 때문에 global slot이 비었을 때 아직 adapter slot을 얻지 못한 ticket이 global head를 선점할 수 있고, 다른 adapter의 실행 가능한 queued run이 불필요하게 막힌다. + +해결 방법: + +- `enqueue()`에서는 adapter gate reservation까지만 수행한다. +- `wait()`에서 adapter gate가 grant된 뒤 global gate를 reserve/wait한다. +- global wait 실패 또는 context/timeout 실패 시 이미 얻은 adapter slot을 반드시 release한다. +- provider queue timeout이 "admission 대기" 전체에 적용되어야 하는지 현재 계약과 테스트 이름에 맞게 명확히 고정한다. 최소한 global wait 때문에 무기한 대기하거나 `max_queue`가 우회되는 동작은 남기지 않는다. + +필수 회귀 테스트: + +- `apps/node/internal/node/node_test.go` +- 시나리오: global concurrency=2, adapter A/B capacity=1, max_queue 충분. +- A1과 B1이 실행 중일 때 A2를 먼저 enqueue하고 B2를 다음 enqueue한다. +- B1만 release하면 B2가 즉시 시작해야 하며, A1 release를 기다리면 실패다. +- global wait에 걸린 run이 queue timeout 정책으로 종료되는 테스트도 추가하거나 기존 timeout 테스트를 확장한다. + +중간 검증: + +```bash +go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Global|TestAdmissionQueue_.*Queue|TestConcurrencyLimit' +``` + +예상: PASS. + +### [REVIEW_REFACTOR-2] Verification Evidence Recovery + +문제: [code_review_cloud_G06_0.log](/config/workspace/iop/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/code_review_cloud_G06_0.log:155)는 `scripts/dev/edge.sh` + `scripts/dev/node.sh` 기반 repo 내부 edge-node 진단/full-cycle 검증을 생략했다. 이 변경은 node 실행/admission path에 닿으므로 보조 `./scripts/e2e-smoke.sh`만으로 완료 증거가 아니다. + +해결 방법: + +- `agent-ops/skills/project/e2e-smoke/SKILL.md` 기준으로 임시 config/포트를 사용해 repo 내부 edge-node 진단을 수행한다. +- edge console에서 `/nodes`, `/capabilities`, `/transport`, 메시지 2회, background run, `/sessions`, `/terminate-session` 결과를 기록한다. +- node local `[node-message]` payload와 edge `[node-*-message]` payload가 run별로 내용/순서까지 동일한지 기록한다. +- 수행 불가 시에는 실제 실행한 명령, stdout/stderr, user-owned blocker 여부, 재개 조건을 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청`에 남긴다. 단순 evidence gap은 사용자 리뷰 요청이 아니라 후속 에이전트가 재시도해야 할 항목이다. + +중간 검증: + +```bash +./scripts/e2e-smoke.sh +``` + +예상: PASS. 단, 이 명령은 보조 smoke이며 full-cycle 대체 증거로 기록하지 않는다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `apps/node/internal/node/run_manager.go` | REVIEW_REFACTOR-1 | +| `apps/node/internal/node/node_test.go` | REVIEW_REFACTOR-1 | +| `agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/CODE_REVIEW-cloud-G06.md` | REVIEW_REFACTOR-2 evidence 기록 | + +## 최종 검증 + +```bash +go test -count=1 ./apps/node/internal/node +go test -race -count=1 ./apps/node/internal/node +go test -count=1 ./apps/node/... +./scripts/e2e-smoke.sh +``` + +추가 필수 검증: `agent-ops/skills/project/e2e-smoke/SKILL.md` 기준의 `scripts/dev/edge.sh` + `scripts/dev/node.sh` repo 내부 edge-node 진단/full-cycle 결과를 `CODE_REVIEW-*-G??.md`에 실제 stdout/stderr 또는 요약 evidence로 기록한다. diff --git a/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log new file mode 100644 index 0000000..3d8dbba --- /dev/null +++ b/agent-task/archive/2026/06/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/plan_cloud_G06_2.log @@ -0,0 +1,86 @@ + + +# Plan - FIFO Admission Queue Timeout Follow-up + +## 이 파일을 읽는 구현 에이전트에게 + +이 계획은 `code_review_cloud_G06_1.log`의 Required 문제만 해결한다. 구현의 마지막 단계는 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것이다. 검증을 실행하고 실제 stdout/stderr를 붙여 넣은 뒤 active 파일을 그대로 둔 채 리뷰 준비 완료를 보고한다. 사용자 결정, 사용자 소유 외부 환경, scope conflict 없이는 사용자에게 직접 묻지 말고, 차단 시 review stub의 `사용자 리뷰 요청` 섹션에 근거를 남긴다. `USER_REVIEW.md`, `complete.log`, log/archive 이동은 code-review 전용이다. + +## 배경 + +2차 구현은 교차 adapter global head-of-line blocking과 full-cycle evidence 문제를 해결했다. 다만 `admissionTicket.mapDeadline`이 parent request deadline과 provider queue timeout deadline을 구분하지 않아, `RunRequest.timeout_sec` 만료도 `queue_timeout`으로 오분류할 수 있다. 특히 `queue_timeout_ms=0`인 provider에서 queued request가 request timeout으로 취소되면 `queue timeout exceeded (queue_timeout=0s)`라는 잘못된 reason이 store/event에 남는다. + +## 사용자 리뷰 요청 흐름 + +구현 중 사용자-only blocker는 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 에이전트는 직접 질문, 채팅 선택지 제시, `request_user_input` 호출을 하지 않는다. code-review가 요청 타당성을 검증하고 실제 `USER_REVIEW.md` 작성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` +- Task ids: + - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. + - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. + - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. +- Completion mode: check-on-pass + +## 범위 결정 근거 + +- 포함: admission wait timeout cause 구분, request timeout while queued 회귀 테스트, 기존 queue timeout/global wait timeout 테스트 유지. +- 제외: 새 public proto reason 추가, Edge/Control Plane 변경, `ProviderSnapshot.Queued` 채우기. + +## 구현 체크리스트 + +- [ ] `RunRequest.timeout_sec` 등 parent deadline이 queue timeout으로 오분류되지 않는 회귀 테스트를 추가한다. +- [ ] provider `queue_timeout_ms` deadline이 실제로 만료된 경우에만 `queue_timeout` reason을 반환하도록 admission wait timeout cause를 분리한다. +- [ ] `queue_timeout_ms=0`인 queued request가 parent deadline으로 취소될 때 store/event error에 `queue_timeout=0s`가 남지 않게 한다. +- [ ] 기존 queue timeout, global-wait timeout, cross-adapter head-of-line tests가 계속 통과하게 한다. +- [ ] 최종 검증 명령을 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 작업 항목 + +### [REVIEW_REVIEW_REFACTOR-1] Timeout Cause Classification + +문제: [run_manager.go](/config/workspace/iop/apps/node/internal/node/run_manager.go:332)의 `mapDeadline`은 `ctx.Err() == context.DeadlineExceeded`인 모든 admission cancellation을 `queue_timeout`으로 변환한다. 하지만 [node.go](/config/workspace/iop/apps/node/internal/node/node.go:141)의 `execCtx`는 `RunRequest.timeout_sec`도 포함하므로, parent request timeout과 provider queue timeout이 같은 reason으로 섞인다. + +해결 방법: + +- queue timeout 전용 cause를 따로 추적한다. 후보: + - `context.WithTimeoutCause(ctx, queueTimeout, errAdmissionQueueTimeout)`를 사용하고 `context.Cause(ctx)`로 queue timeout만 분류한다. + - 또는 adapter/global wait 전체에 공유되는 `time.Timer`/timeout channel을 만들어 그 channel이 선택된 경우에만 `newQueueTimeoutError(queueTimeout)`를 반환한다. +- parent context cancel/deadline은 `queue_timeout`으로 바꾸지 않는다. 현재 public reason을 늘리지 않는다면 기존 `concurrency_unavailable` reason을 유지한다. +- `queue_timeout_ms=0`이면 queue timeout mechanism 자체가 비활성이라는 의미이므로 `queue timeout exceeded (queue_timeout=0s)`가 나오면 안 된다. + +필수 테스트: + +- `apps/node/internal/node/node_test.go` +- `queue_timeout_ms=0`, `max_queue>0`, capacity=1에서 첫 run이 slot을 잡고, 두 번째 foreground run은 `TimeoutSec: 1`로 queued 대기하다 parent request timeout으로 종료된다. +- store error와 반환 error가 `queue timeout` 또는 `queue_timeout=0s`를 포함하지 않는지 확인한다. +- `queue_timeout_ms > TimeoutSec`인 경우에도 parent deadline이 먼저 오면 queue timeout으로 오분류하지 않는 테스트를 추가하거나 위 테스트에 포함한다. +- 기존 `TestAdmissionQueue_QueueTimeout`과 `TestAdmissionQueue_QueueTimeoutAppliesAcrossGates`는 계속 `queue timeout` reason을 확인해야 한다. + +중간 검증: + +```bash +go test -count=1 ./apps/node/internal/node -run 'TestAdmissionQueue_.*Timeout|TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine' +``` + +예상: PASS. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `apps/node/internal/node/run_manager.go` | REVIEW_REVIEW_REFACTOR-1 | +| `apps/node/internal/node/node_test.go` | REVIEW_REVIEW_REFACTOR-1 | + +## 최종 검증 + +```bash +go test -count=1 ./apps/node/internal/node +go test -race -count=1 ./apps/node/internal/node +go test -count=1 ./apps/node/... +./scripts/e2e-smoke.sh +``` + +repo 내부 edge-node 진단 증거는 `code_review_cloud_G06_1.log`에서 이미 회복되었으므로, 이번 후속에서 admission timeout 코드가 바뀌어 smoke가 실패하지 않는지만 확인한다. diff --git a/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/CODE_REVIEW-cloud-G06.md b/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/CODE_REVIEW-cloud-G06.md deleted file mode 100644 index 769c8e2..0000000 --- a/agent-task/m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue/CODE_REVIEW-cloud-G06.md +++ /dev/null @@ -1,127 +0,0 @@ - - -# Code Review Reference - REFACTOR - -> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** -> The task is NOT complete until every implementation-owned section below is filled in. -> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. -> Fill implementation-owned sections, then stop with active files in place and report ready for review. -> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. - -## 개요 - -date=2026-06-14 -task=m-provider-availability-capacity-queue-foundation/04+03_fifo_admission_queue, plan=0, tag=REFACTOR - -## Roadmap Targets - -- Milestone: `agent-roadmap/phase/inference-provider-extension/milestones/provider-availability-capacity-queue-foundation.md` -- Task ids: - - `admission-gate`: Node provider executor가 `in_flight < capacity`일 때만 provider 호출을 시작하고, capacity가 찬 요청은 provider별 FIFO queue에 넣는다. - - `queue-release`: 실행 중 요청이 응답, 실패, 취소, timeout으로 종료되면 `in_flight`를 줄이고 queue의 첫 요청을 실행 슬롯으로 승격한다. - - `queue-reject`: `max_queue` 초과 또는 `queue_timeout` 초과 요청은 명확한 error/rejection reason으로 종료한다. -- Completion mode: check-on-pass - -## 이 파일을 읽는 리뷰 에이전트에게 - -> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. - -각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. - ---- - -## 구현 항목별 완료 여부 - -| 항목 | 완료 여부 | -|------|---------| -| [REFACTOR-1] FIFO Admission Manager | [ ] | -| [REFACTOR-2] OnRunRequest Admission Path | [ ] | -| [REFACTOR-3] Queue Rejection Events | [ ] | - -## 구현 체크리스트 - -- [ ] predecessor `03_queue_capability_contract`의 `complete.log`가 active 또는 archive에 있는지 확인한다. -- [ ] `permitManager` 또는 새 admission manager를 FIFO queue semantics로 교체한다. -- [ ] `OnRunRequest`가 capacity 여유가 있으면 즉시 실행하고, capacity가 차면 provider별 FIFO queue에 요청을 대기시킨다. -- [ ] 실행 완료, 실패, 취소, timeout 시 permit을 release하고 queue head를 실행 슬롯으로 승격한다. -- [ ] `max_queue` 초과와 `queue_timeout_ms` 초과는 store status와 RunEvent error에 명확한 reason을 남긴다. -- [ ] foreground/background run 모두 같은 admission path를 사용한다. -- [ ] node unit/integration tests를 FIFO 정책에 맞게 추가/수정한다. -- [ ] 최종 검증 명령을 실행한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. - -## 코드리뷰 전용 체크리스트 - -> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. - -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G06_N.log`로 아카이브한다. -- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G06_M.log`로 아카이브한다. -- [ ] PASS이면 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. -- [ ] PASS이면 active task 디렉터리를 archive로 이동한다. -- [ ] PASS이고 task group이 `m-provider-availability-capacity-queue-foundation`이면 완료 이벤트 메타데이터를 보고하고 roadmap을 직접 수정하지 않는다. - -## 계획 대비 변경 사항 - -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ - -## 주요 설계 결정 - -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ - -## 사용자 리뷰 요청 - -_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ - -- 상태: 없음 -- 사유 유형: 없음 -- 결정 필요: 없음 -- 차단 근거: 없음 -- 실행한 검증/명령: 없음 -- 자동 후속 불가 이유: 없음 -- 재개 조건: 없음 - -## 리뷰어를 위한 체크포인트 - -- predecessor `03_queue_capability_contract` 완료 근거가 있는지 확인한다. -- queue admission이 FIFO이며 goroutine/context leak 가능성이 없는지 확인한다. -- foreground/background semantics와 store/event side effects가 테스트와 일치하는지 확인한다. -- full-cycle/e2e 생략이 있으면 사유와 남은 위험이 기록됐는지 확인한다. - -## 검증 결과 - -_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ - -### REFACTOR-1 중간 검증 -```bash -$ go test -count=1 ./apps/node/internal/node -run 'Test.*Queue|TestConcurrency' -(output) -``` - -### REFACTOR-2 중간 검증 -```bash -$ go test -count=1 ./apps/node/internal/node -(output) -``` - -### REFACTOR-3 중간 검증 -```bash -$ go test -count=1 ./apps/node/internal/node -run 'Test.*Reject|Test.*Queue|TestConcurrencyLimit_.*Event' -(output) -``` - -### 최종 검증 -```bash -$ go test -count=1 ./apps/node/internal/node -(output) - -$ go test -count=1 ./apps/node/... -(output) - -$ ./scripts/e2e-smoke.sh -(output) -``` - ---- - -> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** diff --git a/apps/node/internal/node/node.go b/apps/node/internal/node/node.go index 187ae6e..c84a7e6 100644 --- a/apps/node/internal/node/node.go +++ b/apps/node/internal/node/node.go @@ -28,15 +28,15 @@ import ( // Node implements transport.Handler and coordinates the full execution pipeline. type Node struct { - nodeID string - router runtime.Router - store *store.Store - runs *runManager - globalPermit *permitManager // node-wide limit across all adapters - adapterPermitsMu sync.Mutex - adapterPermits map[string]*permitManager // per adapter-key limit - out io.Writer - logger *zap.Logger + nodeID string + router runtime.Router + store *store.Store + runs *runManager + globalGate *fifoGate // node-wide FIFO admission across all adapters + adapterGatesMu sync.Mutex + adapterGates map[string]*fifoGate // per adapter-key FIFO admission + out io.Writer + logger *zap.Logger } // New creates a Node. It satisfies transport.Handler. @@ -56,14 +56,14 @@ func New( out = os.Stdout } return &Node{ - nodeID: nodeID, - router: router, - store: st, - runs: newRunManager(), - globalPermit: newPermitManager(globalConcurrency), - adapterPermits: make(map[string]*permitManager), - out: out, - logger: logger, + nodeID: nodeID, + router: router, + store: st, + runs: newRunManager(), + globalGate: newFifoGate(globalConcurrency, -1, 0), + adapterGates: make(map[string]*fifoGate), + out: out, + logger: logger, } } @@ -95,54 +95,49 @@ func (n *Node) OnRunRequest(ctx context.Context, sess *transport.Session, req *i return fmt.Errorf("node: resolve: %w", err) } - // Concurrency gate (foreground and background use the same path). - // Policy: reject-on-exceed (no queue, no wait). + // Admission gate (foreground and background use the same path). + // Policy: provider-scoped FIFO queue. A request runs immediately when + // in_flight < capacity; otherwise it waits in the adapter's FIFO queue and + // is promoted to an execution slot as earlier runs release. Requests are + // rejected when the queue is full (max_queue) or the queue timeout elapses. // - // Two independent limits are enforced: - // 1. global (node-wide): n.globalPermit — shared across all adapter keys. - // 2. adapter-key: per-adapter Capabilities.MaxConcurrency permit. - // - // Both must be acquired. If global is acquired but adapter-key fails, global - // is immediately released. The reject limit reported to the caller is the - // stricter of the two. - adapterCap := 0 - if caps, capsErr := adapter.Capabilities(ctx); capsErr == nil { - adapterCap = caps.MaxConcurrency - } - adapterPM := n.adapterPermitFor(spec.Adapter, adapterCap) - - if !n.globalPermit.tryAcquire() { - limit := int(n.globalPermit.limit) - n.logger.Warn("global concurrency limit exceeded", - zap.String("run_id", spec.RunID), - zap.Int("global_limit", limit), - ) - n.rejectRun(ctx, sess, spec, limit) - return fmt.Errorf("node: run %s: %w", spec.RunID, ErrConcurrencyLimitExceeded) - } - if !adapterPM.tryAcquire() { - n.globalPermit.release() - n.logger.Warn("adapter concurrency limit exceeded", - zap.String("run_id", spec.RunID), - zap.String("adapter", spec.Adapter), - zap.Int("adapter_limit", adapterCap), - ) - n.rejectRun(ctx, sess, spec, adapterCap) - return fmt.Errorf("node: run %s: %w", spec.RunID, ErrConcurrencyLimitExceeded) + // Two gates compose admission: the per-adapter gate (capacity, max_queue, + // queue_timeout from Capabilities) and the node-wide global gate. + var caps runtime.Capabilities + if c, capsErr := adapter.Capabilities(ctx); capsErr == nil { + caps = c } + admission := n.admissionFor(spec.Adapter, caps) + // Record the request as queued before waiting so its lifecycle is + // observable even while it sits in the FIFO queue. if err := n.store.InsertRun(ctx, store.RunRecord{ RunID: spec.RunID, Adapter: spec.Adapter, Target: spec.Target, SessionID: normalizeSessionID(spec.SessionID), Background: spec.Background, - Status: "running", + Status: "queued", CreatedAt: time.Now(), }); err != nil { n.logger.Warn("store: insert run", zap.String("run_id", spec.RunID), zap.Error(err)) } + // Reserve a FIFO position without blocking. Only queue overflow fails here; + // waiting for promotion happens later in wait(). + ticket, err := admission.enqueue() + if err != nil { + n.logger.Warn("run admission rejected", + zap.String("run_id", spec.RunID), + zap.String("adapter", spec.Adapter), + zap.String("reason", string(admissionRejectReason(err))), + ) + n.rejectRun(ctx, sess, spec, err) + return fmt.Errorf("node: run %s: %w", spec.RunID, err) + } + + // The run context governs both the queue wait and execution, so cancelling a + // queued run via OnCancel also removes it from the FIFO queue. execCtx, cancel := context.WithCancel(ctx) if spec.TimeoutSec > 0 { execCtx, cancel = context.WithTimeout(ctx, time.Duration(spec.TimeoutSec)*time.Second) @@ -166,9 +161,21 @@ func (n *Node) OnRunRequest(ctx context.Context, sess *transport.Session, req *i background: spec.Background, } + // admit blocks until the run holds an execution slot, then promotes the + // stored status to running. It returns an admissionError on queue timeout or + // cancellation while queued. + admit := func() error { + if err := ticket.wait(execCtx); err != nil { + return err + } + if err := n.store.MarkRunning(context.Background(), spec.RunID); err != nil { + n.logger.Warn("store: mark running", zap.String("run_id", spec.RunID), zap.Error(err)) + } + return nil + } + run := func() error { - defer n.globalPermit.release() - defer adapterPM.release() + defer ticket.release() defer cancel() defer n.runs.deregister(spec.RunID) defer close(h.done) @@ -178,44 +185,87 @@ func (n *Node) OnRunRequest(ctx context.Context, sess *transport.Session, req *i return execErr } + // launch waits for an execution slot then runs. On admission failure it + // records the rejection. It is invoked synchronously for foreground runs and + // in a goroutine for queued background runs. + launch := func() error { + if err := admit(); err != nil { + n.logger.Warn("run admission rejected", + zap.String("run_id", spec.RunID), + zap.String("adapter", spec.Adapter), + zap.String("reason", string(admissionRejectReason(err))), + ) + cancel() + n.runs.deregister(spec.RunID) + close(h.done) + n.rejectRun(context.Background(), sess, spec, err) + return fmt.Errorf("node: run %s: %w", spec.RunID, err) + } + return run() + } + + // A queued background run returns to the caller after enqueue; promotion and + // execution proceed in a goroutine. All other runs (immediately-admitted + // background, or any foreground run) take the synchronous path. if spec.Background { - go func() { _ = run() }() + go func() { _ = launch() }() return nil } - return run() + return launch() } -// adapterPermitFor returns the shared permitManager for the given adapter -// instance key, creating one with the given limit if not yet seen. -// limit <= 0 means unlimited. -func (n *Node) adapterPermitFor(adapterKey string, limit int) *permitManager { - n.adapterPermitsMu.Lock() - defer n.adapterPermitsMu.Unlock() - pm, ok := n.adapterPermits[adapterKey] - if !ok { - pm = newPermitManager(limit) - n.adapterPermits[adapterKey] = pm +// admissionFor returns an admissionManager pairing the node-wide global gate +// with the per-adapter FIFO gate for the given adapter instance key. The +// adapter gate is created on first use from the adapter's Capabilities +// (MaxConcurrency, MaxQueue, QueueTimeoutMS) and shared across runs. +func (n *Node) admissionFor(adapterKey string, caps runtime.Capabilities) *admissionManager { + return &admissionManager{ + global: n.globalGate, + adapter: n.adapterGateFor(adapterKey, caps), } - return pm +} + +// adapterGateFor returns the shared FIFO gate for the given adapter instance +// key, creating one from caps if not yet seen. MaxConcurrency <= 0 means the +// adapter gate is unlimited. +func (n *Node) adapterGateFor(adapterKey string, caps runtime.Capabilities) *fifoGate { + n.adapterGatesMu.Lock() + defer n.adapterGatesMu.Unlock() + g, ok := n.adapterGates[adapterKey] + if !ok { + g = newFifoGate( + caps.MaxConcurrency, + caps.MaxQueue, + time.Duration(caps.QueueTimeoutMS)*time.Millisecond, + ) + n.adapterGates[adapterKey] = g + } + return g } // rejectRun records a rejected run in the store and sends an error RunEvent to // the session so Edge can observe the rejection through the normal event stream. -func (n *Node) rejectRun(ctx context.Context, sess *transport.Session, spec runtime.ExecutionSpec, limit int) { - errMsg := fmt.Sprintf("concurrency limit exceeded (limit=%d)", limit) +// The rejection message carries the concrete reason (queue_full, queue_timeout, +// concurrency_unavailable) extracted from the admission error. +func (n *Node) rejectRun(ctx context.Context, sess *transport.Session, spec runtime.ExecutionSpec, admitErr error) { + errMsg := admissionMessage(admitErr) - if err := n.store.InsertRun(ctx, store.RunRecord{ - RunID: spec.RunID, - Adapter: spec.Adapter, - Target: spec.Target, - SessionID: normalizeSessionID(spec.SessionID), - Background: spec.Background, - Status: "rejected", - CreatedAt: time.Now(), - }); err != nil { - n.logger.Warn("store: insert rejected run", zap.String("run_id", spec.RunID), zap.Error(err)) - } else if err := n.store.CompleteRun(ctx, spec.RunID, "rejected", errMsg); err != nil { + // The run was inserted as "queued" before admission; mark it rejected. + // If the insert never happened (e.g. it was skipped on error), fall back to + // inserting a terminal rejected record so the rejection is still observable. + if err := n.store.CompleteRun(ctx, spec.RunID, "rejected", errMsg); err != nil { n.logger.Warn("store: complete rejected run", zap.String("run_id", spec.RunID), zap.Error(err)) + if insErr := n.store.InsertRun(ctx, store.RunRecord{ + RunID: spec.RunID, + Adapter: spec.Adapter, + Target: spec.Target, + SessionID: normalizeSessionID(spec.SessionID), + Background: spec.Background, + Status: "rejected", + CreatedAt: time.Now(), + }); insErr != nil { + n.logger.Warn("store: insert rejected run", zap.String("run_id", spec.RunID), zap.Error(insErr)) + } } if sess != nil && sess.IsAlive() { @@ -333,12 +383,12 @@ func (n *Node) handleCapabilitiesCommand(ctx context.Context, req *iop.NodeComma sort.Strings(targets) - n.adapterPermitsMu.Lock() - pm, ok := n.adapterPermits[req.GetAdapter()] - n.adapterPermitsMu.Unlock() + n.adapterGatesMu.Lock() + gate, ok := n.adapterGates[req.GetAdapter()] + n.adapterGatesMu.Unlock() inFlight := 0 if ok { - inFlight = pm.activeCount() + inFlight = gate.activeCount() } result := map[string]string{ diff --git a/apps/node/internal/node/node_concurrency_integration_test.go b/apps/node/internal/node/node_concurrency_integration_test.go index 7fa3f31..0865ab5 100644 --- a/apps/node/internal/node/node_concurrency_integration_test.go +++ b/apps/node/internal/node/node_concurrency_integration_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "strings" "testing" "time" @@ -87,9 +88,10 @@ func startMockEdge(t *testing.T, ctx context.Context, listenAddr string) *toki.T } } -// TestConcurrencyLimit_RejectEventObservedByEdge is an integration test that -// verifies the RunEvent{type:"error"} sent by rejectRun() actually arrives at -// the mock Edge server when the Node's concurrency limit is exceeded. +// TestQueueOverflow_RejectEventObservedByEdge is an integration test that +// verifies the RunEvent{type:"error"} sent by rejectRun() carries the queue +// overflow reason and actually arrives at the mock Edge server when the Node's +// FIFO admission queue is full (capacity=1, max_queue=0). // // Setup: // 1. Start mock Edge TCP server. @@ -98,8 +100,9 @@ func startMockEdge(t *testing.T, ctx context.Context, listenAddr string) *toki.T // 4. Attach the session to the node via SetHandler. // 5. From the Edge side, send a first RunRequest whose adapter blocks. // 6. After the first run starts, send a second RunRequest from the Edge side. -// 7. Assert that a RunEvent{type:"error", run_id:} arrives at the Edge. -func TestConcurrencyLimit_RejectEventObservedByEdge(t *testing.T) { +// 7. Assert a RunEvent{type:"error", run_id:} with a queue_full reason +// arrives at the Edge. +func TestQueueOverflow_RejectEventObservedByEdge(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -158,7 +161,8 @@ func TestConcurrencyLimit_RejectEventObservedByEdge(t *testing.T) { }) // Build real node.Node with global concurrency=1. - sa := newSlowAdapter() // MaxConcurrency=1; blocks until sa.release is closed + // capacity=1, max_queue=0 → a second concurrent run overflows the queue. + sa := newQueuedSlowAdapter("slow", 1, 0, 0) rtr := &fixedRouter{ adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}, @@ -178,14 +182,11 @@ func TestConcurrencyLimit_RejectEventObservedByEdge(t *testing.T) { t.Fatalf("send first run request: %v", err) } - // Wait until the first run's adapter has started executing so the permit is held. - select { - case <-sa.started: - case <-time.After(3 * time.Second): - t.Fatal("first run never started") - } + // Wait until the first run's adapter has started executing so the slot is held. + waitStarted(t, sa, "run-hold-1") - // Send second RunRequest; should be rejected because global=1 is taken. + // Send second RunRequest; should be rejected because capacity=1 is taken and + // the queue (max_queue=0) cannot hold it. secondReq := &iop.RunRequest{RunId: "run-reject-2", Adapter: "slow", Target: "v1"} if err := edgeClient.Send(secondReq); err != nil { t.Fatalf("send second run request: %v", err) @@ -200,8 +201,11 @@ func TestConcurrencyLimit_RejectEventObservedByEdge(t *testing.T) { if ev.GetError() == "" { t.Fatal("reject RunEvent has empty error field") } + if !strings.Contains(ev.GetError(), "queue full") { + t.Fatalf("expected queue_full reason in reject event, got %q", ev.GetError()) + } // Success: RunEvent{type:"error", run_id:"run-reject-2"} observed. - close(sa.release) + sa.releaseRun("run-hold-1") return } case <-deadline: diff --git a/apps/node/internal/node/node_test.go b/apps/node/internal/node/node_test.go index 33c648f..8c52b26 100644 --- a/apps/node/internal/node/node_test.go +++ b/apps/node/internal/node/node_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "strings" + "sync" "sync/atomic" "testing" "time" @@ -1207,23 +1208,8 @@ func TestOnCommandRequest_Capabilities_ExactInstanceKey(t *testing.T) { // --- concurrency gate tests --- -// slowAdapter blocks until released, exposing a start channel so tests can -// synchronize. Execute returns nil on normal completion. -type slowAdapter struct { - started chan struct{} - release chan struct{} -} - -func newSlowAdapter() *slowAdapter { - return &slowAdapter{started: make(chan struct{}, 1), release: make(chan struct{})} -} - -func (a *slowAdapter) Name() string { return "slow" } -func (a *slowAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { - return runtime.Capabilities{AdapterName: "slow", MaxConcurrency: 1}, nil -} - -// slowAdapterUnlimited is like slowAdapter but reports MaxConcurrency=0 (unlimited). +// slowAdapterUnlimited reports MaxConcurrency=0 (unlimited) and blocks until +// released, exposing a start channel so tests can synchronize. type slowAdapterUnlimited struct { started chan struct{} release chan struct{} @@ -1250,149 +1236,429 @@ func (a *slowAdapterUnlimited) Execute(ctx context.Context, _ runtime.ExecutionS return nil } -// slowHighCapAdapter is like slowAdapter but reports MaxConcurrency=4, -// used to verify that the global limit overrides a looser adapter cap. -type slowHighCapAdapter struct { - started chan struct{} - release chan struct{} +// queuedSlowAdapter is a configurable adapter used by the FIFO admission tests. +// It advertises MaxConcurrency / MaxQueue / QueueTimeout via Capabilities, and +// gives per-run control: each Execute blocks until the test releases (or fails) +// that specific run id, so tests can observe queue ordering deterministically. +type queuedSlowAdapter struct { + name string + maxConcurrency int + maxQueue int + queueTimeout time.Duration + + mu sync.Mutex + gates map[string]chan error // run_id → release signal carrying exec result + startSeq chan string // run_id pushed when Execute begins } -func newSlowHighCapAdapter() *slowHighCapAdapter { - return &slowHighCapAdapter{started: make(chan struct{}, 1), release: make(chan struct{})} -} - -func (a *slowHighCapAdapter) Name() string { return "slow-hicap" } -func (a *slowHighCapAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { - return runtime.Capabilities{AdapterName: "slow-hicap", MaxConcurrency: 4}, nil -} -func (a *slowHighCapAdapter) Execute(ctx context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { - select { - case a.started <- struct{}{}: - default: +func newQueuedSlowAdapter(name string, maxConcurrency, maxQueue int, queueTimeout time.Duration) *queuedSlowAdapter { + return &queuedSlowAdapter{ + name: name, + maxConcurrency: maxConcurrency, + maxQueue: maxQueue, + queueTimeout: queueTimeout, + gates: make(map[string]chan error), + startSeq: make(chan string, 64), } +} + +func (a *queuedSlowAdapter) Name() string { return a.name } + +func (a *queuedSlowAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { + return runtime.Capabilities{ + AdapterName: a.name, + MaxConcurrency: a.maxConcurrency, + MaxQueue: a.maxQueue, + QueueTimeoutMS: int(a.queueTimeout / time.Millisecond), + }, nil +} + +func (a *queuedSlowAdapter) gateFor(runID string) chan error { + a.mu.Lock() + defer a.mu.Unlock() + ch, ok := a.gates[runID] + if !ok { + ch = make(chan error, 1) + a.gates[runID] = ch + } + return ch +} + +func (a *queuedSlowAdapter) Execute(ctx context.Context, spec runtime.ExecutionSpec, _ runtime.EventSink) error { + gate := a.gateFor(spec.RunID) + a.startSeq <- spec.RunID select { - case <-a.release: + case res := <-gate: + return res case <-ctx.Done(): return runtime.ErrRunCancelled } - return nil } -func (a *slowAdapter) Execute(ctx context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { - select { - case a.started <- struct{}{}: - default: +// releaseRun lets the given run complete successfully. +func (a *queuedSlowAdapter) releaseRun(runID string) { a.gateFor(runID) <- nil } + +// failRun lets the given run terminate with a non-cancel error. +func (a *queuedSlowAdapter) failRun(runID string) { a.gateFor(runID) <- errBoom } + +// preRelease pre-arms a run so it completes the instant it starts executing. +func (a *queuedSlowAdapter) preRelease(runID string) { a.gateFor(runID) <- nil } + +var errBoom = errors.New("boom") + +// waitStarted blocks until the adapter reports the given run id started, or +// fails the test after a timeout. +func waitStarted(t *testing.T, a *queuedSlowAdapter, runID string) { + t.Helper() + deadline := time.After(2 * time.Second) + for { + select { + case got := <-a.startSeq: + if got == runID { + return + } + // Not the one we want; requeue for other waiters. + a.startSeq <- got + time.Sleep(2 * time.Millisecond) + case <-deadline: + t.Fatalf("run %s never started", runID) + } } - select { - case <-a.release: - case <-ctx.Done(): - return runtime.ErrRunCancelled - } - return nil } -// TestConcurrencyLimit_RejectSecondForeground verifies that a second foreground -// run is rejected immediately when the global limit is 1 and one run is active. -func TestConcurrencyLimit_RejectSecondForeground(t *testing.T) { - sa := newSlowAdapter() +// waitAnyStarted returns the next run id that began executing. +func waitAnyStarted(t *testing.T, a *queuedSlowAdapter) string { + t.Helper() + select { + case got := <-a.startSeq: + return got + case <-time.After(2 * time.Second): + t.Fatal("no run started within timeout") + return "" + } +} + +// requireQueued asserts the run is stored with status "queued" (polling briefly +// for the asynchronous store write). +func requireQueued(t *testing.T, st *store.Store, runID string) { + t.Helper() + requireStatusEventually(t, st, runID, "queued") +} + +// requireStatus asserts the run is stored with exactly the given status now. +func requireStatus(t *testing.T, st *store.Store, runID, status string) *store.RunRecord { + t.Helper() + return requireStatusEventually(t, st, runID, status) +} + +// requireStatusEventually polls until the run reaches the expected status. +func requireStatusEventually(t *testing.T, st *store.Store, runID, status string) *store.RunRecord { + t.Helper() + deadline := time.After(2 * time.Second) + for { + run, err := st.GetRun(context.Background(), runID) + if err != nil { + t.Fatalf("GetRun(%s): %v", runID, err) + } + if run != nil && run.Status == status { + return run + } + select { + case <-deadline: + got := "" + if run != nil { + got = run.Status + } + t.Fatalf("run %s: expected status %q, got %q", runID, status, got) + case <-time.After(5 * time.Millisecond): + } + } +} + +// TestAdmissionQueue_ForegroundSecondWaitsThenRuns verifies that a second +// foreground run is not rejected: it waits in the FIFO queue and executes once +// the first run releases its slot. +func TestAdmissionQueue_ForegroundSecondWaitsThenRuns(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} - n, _ := makeNodeWithConcurrency(t, router, 1) + n, st := makeNodeWithConcurrency(t, router, 1) - errc := make(chan error, 1) + first := make(chan error, 1) go func() { - errc <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + first <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ RunId: "run-fg-1", Adapter: "slow", Target: "v1", }) }() + waitStarted(t, sa, "run-fg-1") - // Wait until the first run has started executing. + second := make(chan error, 1) + go func() { + second <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-fg-2", Adapter: "slow", Target: "v1", + }) + }() + + // The second run must remain queued (not started, not rejected) while the + // first holds the only slot. + requireQueued(t, st, "run-fg-2") select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("first run never started") + case err := <-second: + t.Fatalf("second run should be queued, but returned early: %v", err) + case <-time.After(150 * time.Millisecond): } - // Second run must be rejected. + // Release the first; the second must then start and complete. + sa.releaseRun("run-fg-1") + if err := <-first; err != nil { + t.Fatalf("first run: %v", err) + } + waitStarted(t, sa, "run-fg-2") + sa.releaseRun("run-fg-2") + if err := <-second; err != nil { + t.Fatalf("second run should succeed after promotion, got %v", err) + } +} + +// TestAdmissionQueue_BackgroundQueuedReturnsThenRuns verifies that a background +// run that cannot run immediately returns after enqueue and is later promoted. +func TestAdmissionQueue_BackgroundQueuedReturnsThenRuns(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + n, st := makeNodeWithConcurrency(t, router, 1) + + first := make(chan error, 1) + go func() { + first <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-bg-hold", Adapter: "slow", Target: "v1", + }) + }() + waitStarted(t, sa, "run-bg-hold") + + // Background queued run returns promptly after enqueue. + if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-bg-2", Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("background queued run should enqueue without error, got %v", err) + } + requireQueued(t, st, "run-bg-2") + + // Release the holder; the queued background run must be promoted and run. + sa.releaseRun("run-bg-hold") + if err := <-first; err != nil { + t.Fatalf("hold run: %v", err) + } + waitStarted(t, sa, "run-bg-2") + sa.releaseRun("run-bg-2") + requireStatusEventually(t, st, "run-bg-2", "completed") +} + +// TestAdmissionQueue_FIFOOrdering verifies that queued runs are promoted in +// first-in-first-out order. +func TestAdmissionQueue_FIFOOrdering(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 8, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + n, _ := makeNodeWithConcurrency(t, router, 1) + + // First run holds the single slot. + hold := make(chan error, 1) + go func() { + hold <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "fifo-0", Adapter: "slow", Target: "v1", Background: true, + }) + }() + waitStarted(t, sa, "fifo-0") + + // Enqueue three background runs in a deterministic order. Each enqueue + // returns before the next is submitted, so queue order is well-defined. + order := []string{"fifo-1", "fifo-2", "fifo-3"} + for _, id := range order { + if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: id, Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("enqueue %s: %v", id, err) + } + } + + // Release runs one at a time and assert they start in FIFO order. + sa.releaseRun("fifo-0") + if err := <-hold; err != nil { + t.Fatalf("fifo-0: %v", err) + } + for _, id := range order { + got := waitAnyStarted(t, sa) + if got != id { + t.Fatalf("FIFO order violated: expected %s to start next, got %s", id, got) + } + sa.releaseRun(id) + } +} + +// TestAdmissionQueue_MaxQueueOverflow verifies that a run is rejected with +// reason queue_full when the FIFO queue is already at max_queue. +func TestAdmissionQueue_MaxQueueOverflow(t *testing.T) { + // capacity=1, max_queue=1: one running + one queued is the ceiling. + sa := newQueuedSlowAdapter("slow", 1, 1, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + n, st := makeNodeWithConcurrency(t, router, 0) // global unlimited; adapter cap governs + + hold := make(chan error, 1) + go func() { + hold <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "of-hold", Adapter: "slow", Target: "v1", Background: true, + }) + }() + waitStarted(t, sa, "of-hold") + + // Fill the single queue slot. + if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "of-queued", Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("first queued run should enqueue, got %v", err) + } + requireQueued(t, st, "of-queued") + + // Third run overflows the queue and must be rejected with queue_full. err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-fg-2", Adapter: "slow", Target: "v1", + RunId: "of-reject", Adapter: "slow", Target: "v1", }) if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { t.Fatalf("expected ErrConcurrencyLimitExceeded, got %v", err) } - - // Release the first run and drain it. - close(sa.release) - if err := <-errc; err != nil { - t.Fatalf("first run: %v", err) + rec := requireStatus(t, st, "of-reject", "rejected") + if !strings.Contains(rec.Error, "queue full") { + t.Fatalf("expected queue_full reason in error, got %q", rec.Error) } + + sa.releaseRun("of-hold") + if err := <-hold; err != nil { + t.Fatalf("of-hold: %v", err) + } + waitStarted(t, sa, "of-queued") + sa.releaseRun("of-queued") } -// TestConcurrencyLimit_RejectBackgroundRun verifies that a background run is -// also subject to the global concurrency gate. -func TestConcurrencyLimit_RejectBackgroundRun(t *testing.T) { - sa := newSlowAdapter() +// TestAdmissionQueue_QueueTimeout verifies that a queued run is rejected with +// reason queue_timeout when it waits past queue_timeout_ms. +func TestAdmissionQueue_QueueTimeout(t *testing.T) { + // capacity=1, max_queue=4, queue_timeout=120ms. + sa := newQueuedSlowAdapter("slow", 1, 4, 120*time.Millisecond) router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} - n, _ := makeNodeWithConcurrency(t, router, 1) + n, st := makeNodeWithConcurrency(t, router, 0) - errc := make(chan error, 1) + hold := make(chan error, 1) go func() { - errc <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-bg-hold", Adapter: "slow", Target: "v1", + hold <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "to-hold", Adapter: "slow", Target: "v1", Background: true, }) }() + waitStarted(t, sa, "to-hold") - select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("hold run never started") - } - - // Background run must also be rejected. + // This foreground run will wait in the queue and time out (holder is never + // released until after the timeout). err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-bg-2", Adapter: "slow", Target: "v1", Background: true, + RunId: "to-timeout", Adapter: "slow", Target: "v1", }) if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { - t.Fatalf("expected ErrConcurrencyLimitExceeded for background run, got %v", err) + t.Fatalf("expected ErrConcurrencyLimitExceeded on queue timeout, got %v", err) + } + rec := requireStatus(t, st, "to-timeout", "rejected") + if !strings.Contains(rec.Error, "queue timeout") { + t.Fatalf("expected queue_timeout reason in error, got %q", rec.Error) } - close(sa.release) - if err := <-errc; err != nil { - t.Fatalf("hold run: %v", err) + sa.releaseRun("to-hold") + if err := <-hold; err != nil { + t.Fatalf("to-hold: %v", err) } } -// TestConcurrencyLimit_PermitReleasedAfterCompletion verifies that after the -// first run completes, a subsequent run can acquire the permit. -func TestConcurrencyLimit_PermitReleasedAfterCompletion(t *testing.T) { - sa := newSlowAdapter() +// TestAdmissionQueue_ReleaseAfterFailurePromotesNext verifies that a queued run +// is promoted when the running run fails. +func TestAdmissionQueue_ReleaseAfterFailurePromotesNext(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + n, st := makeNodeWithConcurrency(t, router, 1) + + first := make(chan error, 1) + go func() { + first <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "fail-1", Adapter: "slow", Target: "v1", + }) + }() + waitStarted(t, sa, "fail-1") + + if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "fail-next", Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("enqueue fail-next: %v", err) + } + requireQueued(t, st, "fail-next") + + // First run fails; its slot must promote the queued run. + sa.failRun("fail-1") + <-first + waitStarted(t, sa, "fail-next") + sa.releaseRun("fail-next") + requireStatusEventually(t, st, "fail-next", "completed") +} + +// TestAdmissionQueue_ReleaseAfterCancelPromotesNext verifies that a queued run +// is promoted when the running run is cancelled. +func TestAdmissionQueue_ReleaseAfterCancelPromotesNext(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + n, st := makeNodeWithConcurrency(t, router, 1) + + first := make(chan error, 1) + go func() { + first <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "cancel-1", Adapter: "slow", Target: "v1", Background: true, + }) + }() + waitStarted(t, sa, "cancel-1") + + if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "cancel-next", Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("enqueue cancel-next: %v", err) + } + requireQueued(t, st, "cancel-next") + <-first + + // Cancel the running run; its slot must promote the queued run. + if err := n.OnCancel(context.Background(), &transport.Session{}, &iop.CancelRequest{RunId: "cancel-1"}); err != nil { + t.Fatalf("cancel: %v", err) + } + waitStarted(t, sa, "cancel-next") + sa.releaseRun("cancel-next") + requireStatusEventually(t, st, "cancel-next", "completed") +} + +// TestAdmissionQueue_PermitReleasedAfterCompletion verifies that after the first +// run completes, a subsequent run can acquire the slot without queuing. +func TestAdmissionQueue_PermitReleasedAfterCompletion(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} n, _ := makeNodeWithConcurrency(t, router, 1) - // First run: start, wait, release. - errc := make(chan error, 1) + first := make(chan error, 1) go func() { - errc <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-seq-1", Adapter: "slow", Target: "v1", + first <- n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "rel-1", Adapter: "slow", Target: "v1", }) }() - select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("seq-1 never started") - } - close(sa.release) - if err := <-errc; err != nil { - t.Fatalf("seq-1: %v", err) + waitStarted(t, sa, "rel-1") + sa.releaseRun("rel-1") + if err := <-first; err != nil { + t.Fatalf("rel-1: %v", err) } - // Second run must succeed now that the permit is free. - sa2 := newSlowAdapter() - router.adapters["slow"] = sa2 - close(sa2.release) // let it complete immediately + // Second run must run immediately now that the slot is free. + sa.preRelease("rel-2") if err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-seq-2", Adapter: "slow", Target: "v1", + RunId: "rel-2", Adapter: "slow", Target: "v1", }); err != nil { - t.Fatalf("seq-2 should succeed after permit released, got %v", err) + t.Fatalf("rel-2 should succeed after slot released, got %v", err) } } @@ -1424,81 +1690,76 @@ func TestConcurrencyLimit_Unlimited(t *testing.T) { } } -// TestConcurrencyLimit_AdapterCapFallback verifies that when global=0 and adapter -// cap=1, the adapter cap is used as the effective limit. -func TestConcurrencyLimit_AdapterCapFallback(t *testing.T) { - sa := newSlowAdapter() // MaxConcurrency=1 +// TestAdmissionQueue_AdapterCapQueues verifies that when global=0 and adapter +// cap=1, a second run queues (rather than being rejected) and runs after release. +func TestAdmissionQueue_AdapterCapQueues(t *testing.T) { + sa := newQueuedSlowAdapter("slow", 1, 4, 0) // adapter cap=1 router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} - nd, _ := makeNodeWithConcurrency(t, router, 0) // global=0 → use adapter cap + nd, st := makeNodeWithConcurrency(t, router, 0) // global=0 → adapter cap governs - errc := make(chan error, 1) + first := make(chan error, 1) go func() { - errc <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-cap-1", Adapter: "slow", Target: "v1", + first <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "cap-1", Adapter: "slow", Target: "v1", }) }() + waitStarted(t, sa, "cap-1") - select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("run-cap-1 never started") + if err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "cap-2", Adapter: "slow", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("cap-2 should enqueue, got %v", err) } + requireQueued(t, st, "cap-2") - // Second run must be rejected by adapter cap. - err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-cap-2", Adapter: "slow", Target: "v1", - }) - if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { - t.Fatalf("expected ErrConcurrencyLimitExceeded from adapter cap, got %v", err) - } - - close(sa.release) - if err := <-errc; err != nil { - t.Fatalf("run-cap-1: %v", err) + sa.releaseRun("cap-1") + if err := <-first; err != nil { + t.Fatalf("cap-1: %v", err) } + waitStarted(t, sa, "cap-2") + sa.releaseRun("cap-2") + requireStatusEventually(t, st, "cap-2", "completed") } -// TestConcurrencyLimit_GlobalBeatsAdapterCap verifies that when both global and -// adapter cap are > 0 and global < adapter cap, the global (stricter) limit applies. -func TestConcurrencyLimit_GlobalBeatsAdapterCap(t *testing.T) { - // slowHighCapAdapter reports MaxConcurrency=4; global=1 → effective=1. - sa := newSlowHighCapAdapter() +// TestAdmissionQueue_GlobalBeatsAdapterCap verifies that the global gate (=1) +// limits effective in-flight to 1 even when the adapter cap is looser (=4): the +// second run queues behind the global slot. +func TestAdmissionQueue_GlobalBeatsAdapterCap(t *testing.T) { + sa := newQueuedSlowAdapter("slow-hicap", 4, 4, 0) // adapter cap=4 router := &fixedRouter{adapterName: "slow-hicap", adapters: map[string]runtime.Adapter{"slow-hicap": sa}} - nd, _ := makeNodeWithConcurrency(t, router, 1) // global=1, adapter cap=4 → effective=1 + nd, st := makeNodeWithConcurrency(t, router, 1) // global=1 → effective=1 - errc := make(chan error, 1) + first := make(chan error, 1) go func() { - errc <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-g1-1", Adapter: "slow-hicap", Target: "v1", + first <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "g1-1", Adapter: "slow-hicap", Target: "v1", }) }() + waitStarted(t, sa, "g1-1") - select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("run-g1-1 never started") + // Second run queues behind the global gate even though adapter cap=4. + if err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "g1-2", Adapter: "slow-hicap", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("g1-2 should enqueue, got %v", err) } + requireQueued(t, st, "g1-2") - // Second run must be rejected by global limit=1 even though adapter cap=4. - err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-g1-2", Adapter: "slow-hicap", Target: "v1", - }) - if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { - t.Fatalf("expected ErrConcurrencyLimitExceeded (global=1 beats adapter cap=4), got %v", err) - } - - close(sa.release) - if err := <-errc; err != nil { - t.Fatalf("run-g1-1: %v", err) + sa.releaseRun("g1-1") + if err := <-first; err != nil { + t.Fatalf("g1-1: %v", err) } + waitStarted(t, sa, "g1-2") + sa.releaseRun("g1-2") + requireStatusEventually(t, st, "g1-2", "completed") } -// TestConcurrencyLimit_GlobalBeatsAdapterCap_DifferentAdapters verifies that -// global=1 rejects a run on adapter B when adapter A already holds the global -// permit, even though both adapter caps are > 1. -func TestConcurrencyLimit_GlobalBeatsAdapterCap_DifferentAdapters(t *testing.T) { - saA := newSlowHighCapAdapter() // adapter "slow-hicap-a", MaxConcurrency=4 - saB := newSlowHighCapAdapter() // adapter "slow-hicap-b", MaxConcurrency=4 +// TestAdmissionQueue_GlobalBeatsAdapterCap_DifferentAdapters verifies that +// global=1 makes a run on adapter B queue while adapter A holds the global slot, +// even though both adapter caps are > 1. +func TestAdmissionQueue_GlobalBeatsAdapterCap_DifferentAdapters(t *testing.T) { + saA := newQueuedSlowAdapter("slow-hicap-a", 4, 4, 0) + saB := newQueuedSlowAdapter("slow-hicap-b", 4, 4, 0) router := &fixedRouter{ adapterName: "slow-hicap-a", @@ -1507,56 +1768,258 @@ func TestConcurrencyLimit_GlobalBeatsAdapterCap_DifferentAdapters(t *testing.T) "slow-hicap-b": saB, }, } - nd, _ := makeNodeWithConcurrency(t, router, 1) // global=1 + nd, st := makeNodeWithConcurrency(t, router, 1) // global=1 - // First run on adapter A: hold the global permit. - errc := make(chan error, 1) + first := make(chan error, 1) go func() { - errc <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-xa-1", Adapter: "slow-hicap-a", Target: "v1", + first <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "xa-1", Adapter: "slow-hicap-a", Target: "v1", }) }() + waitStarted(t, saA, "xa-1") + + // Run on adapter B queues because the global slot is taken by adapter A. + router.adapterName = "slow-hicap-b" + if err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "xb-1", Adapter: "slow-hicap-b", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("xb-1 should enqueue, got %v", err) + } + requireQueued(t, st, "xb-1") + + sa := saA + sa.releaseRun("xa-1") + if err := <-first; err != nil { + t.Fatalf("xa-1: %v", err) + } + waitStarted(t, saB, "xb-1") + saB.releaseRun("xb-1") + requireStatusEventually(t, st, "xb-1", "completed") +} + +// TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine is the regression test for +// the gate-composition fix: a queued run on one adapter must not occupy a global +// slot it cannot yet use and thereby block a runnable queue head on a different +// adapter. +// +// Setup: global=2, adapter A capacity=1, adapter B capacity=1. +// - A1 and B1 run, consuming both global slots and both adapter slots. +// - A2 is enqueued first (waiting on adapter A slot, held by A1). +// - B2 is enqueued next (waiting on adapter B slot, held by B1). +// - Releasing only B1 must let B2 start immediately, because A2 (still without +// an adapter A slot) must NOT have grabbed the freed global slot ahead of it. +func TestAdmissionQueue_GlobalNoCrossAdapterHeadOfLine(t *testing.T) { + saA := newQueuedSlowAdapter("adapter-a", 1, 4, 0) + saB := newQueuedSlowAdapter("adapter-b", 1, 4, 0) + router := &fixedRouter{ + adapterName: "adapter-a", + adapters: map[string]runtime.Adapter{ + "adapter-a": saA, + "adapter-b": saB, + }, + } + nd, st := makeNodeWithConcurrency(t, router, 2) // global=2 + + // A1 and B1 occupy both global slots and both adapter slots. + a1 := make(chan error, 1) + go func() { + a1 <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "a1", Adapter: "adapter-a", Target: "v1", Background: true, + }) + }() + waitStarted(t, saA, "a1") + + router.adapterName = "adapter-b" + b1 := make(chan error, 1) + go func() { + b1 <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "b1", Adapter: "adapter-b", Target: "v1", Background: true, + }) + }() + waitStarted(t, saB, "b1") + + // Enqueue A2 first (waits on adapter A slot), then B2 (waits on adapter B slot). + router.adapterName = "adapter-a" + if err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "a2", Adapter: "adapter-a", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("enqueue a2: %v", err) + } + requireQueued(t, st, "a2") + + router.adapterName = "adapter-b" + if err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "b2", Adapter: "adapter-b", Target: "v1", Background: true, + }); err != nil { + t.Fatalf("enqueue b2: %v", err) + } + requireQueued(t, st, "b2") + + // Release only B1: its adapter B slot and one global slot free up. B2 must + // start. A2 stays queued because adapter A slot (a1) is still held. + <-b1 + saB.releaseRun("b1") + waitStarted(t, saB, "b2") // fails here if A2 head-of-line-blocked the global slot + + // A2 must still be queued (it has no adapter A slot yet). + requireStatus(t, st, "a2", "queued") select { - case <-saA.started: - case <-time.After(2 * time.Second): - t.Fatal("run-xa-1 never started") + case <-saA.startSeq: + t.Fatal("a2 must not start before a1 releases its adapter slot") + case <-time.After(100 * time.Millisecond): } - // Second run on adapter B must be rejected because global permit is taken. - router.adapterName = "slow-hicap-b" + // Now release a1 → A2 can take adapter A slot and the remaining global slot. + <-a1 + saA.releaseRun("a1") + waitStarted(t, saA, "a2") + + saB.releaseRun("b2") + saA.releaseRun("a2") + requireStatusEventually(t, st, "a2", "completed") + requireStatusEventually(t, st, "b2", "completed") +} + +// TestAdmissionQueue_QueueTimeoutAppliesAcrossGates verifies that the queue +// timeout bounds the total admission wait even when the run is blocked waiting +// for the global slot after acquiring its adapter slot. +func TestAdmissionQueue_QueueTimeoutAppliesAcrossGates(t *testing.T) { + // adapter A and B each capacity=1 with a queue timeout; global=1 is the + // bottleneck so the second adapter's run holds its adapter slot but blocks on + // the global gate, then must time out. + saA := newQueuedSlowAdapter("adapter-a", 1, 4, 120*time.Millisecond) + saB := newQueuedSlowAdapter("adapter-b", 1, 4, 120*time.Millisecond) + router := &fixedRouter{ + adapterName: "adapter-a", + adapters: map[string]runtime.Adapter{ + "adapter-a": saA, + "adapter-b": saB, + }, + } + nd, st := makeNodeWithConcurrency(t, router, 1) // global=1 + + a1 := make(chan error, 1) + go func() { + a1 <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "ta1", Adapter: "adapter-a", Target: "v1", Background: true, + }) + }() + waitStarted(t, saA, "ta1") + + // Run on adapter B acquires adapter B slot immediately but blocks on global=1. + // It must time out via the queue timeout rather than waiting indefinitely. + router.adapterName = "adapter-b" err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-xb-1", Adapter: "slow-hicap-b", Target: "v1", + RunId: "tb1", Adapter: "adapter-b", Target: "v1", }) if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { - t.Fatalf("expected ErrConcurrencyLimitExceeded for cross-adapter second run, got %v", err) + t.Fatalf("expected ErrConcurrencyLimitExceeded on global-wait timeout, got %v", err) + } + rec := requireStatus(t, st, "tb1", "rejected") + if !strings.Contains(rec.Error, "queue timeout") { + t.Fatalf("expected queue_timeout reason for global-wait timeout, got %q", rec.Error) } - close(saA.release) - if err := <-errc; err != nil { - t.Fatalf("run-xa-1: %v", err) + <-a1 + saA.releaseRun("ta1") +} + +// TestAdmissionQueue_ParentTimeoutNotQueueTimeout verifies that when a queued +// run is terminated by its parent RunRequest.timeout_sec (not the provider +// queue timeout), the rejection reason is NOT misreported as queue_timeout. With +// queue_timeout_ms=0 the queue timeout mechanism is disabled, so the bogus +// "queue timeout exceeded (queue_timeout=0s)" message must never appear. +func TestAdmissionQueue_ParentTimeoutNotQueueTimeout(t *testing.T) { + // queue_timeout_ms=0 (disabled), max_queue=4, capacity=1. + sa := newQueuedSlowAdapter("slow", 1, 4, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + nd, st := makeNodeWithConcurrency(t, router, 0) + + hold := make(chan error, 1) + go func() { + hold <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "pt-hold", Adapter: "slow", Target: "v1", Background: true, + }) + }() + waitStarted(t, sa, "pt-hold") + + // Second foreground run queues (queue enabled, slot busy) and is terminated + // by its own 1s request timeout, not by a queue timeout. + err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "pt-timeout", Adapter: "slow", Target: "v1", TimeoutSec: 1, + }) + if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { + t.Fatalf("expected ErrConcurrencyLimitExceeded on parent request timeout, got %v", err) + } + if strings.Contains(err.Error(), "queue timeout") { + t.Fatalf("parent request timeout must not be reported as queue timeout, got %q", err.Error()) + } + rec := requireStatus(t, st, "pt-timeout", "rejected") + if strings.Contains(rec.Error, "queue timeout") || strings.Contains(rec.Error, "queue_timeout=0s") { + t.Fatalf("store error must not contain a queue_timeout reason, got %q", rec.Error) + } + + sa.releaseRun("pt-hold") + if err := <-hold; err != nil { + t.Fatalf("pt-hold: %v", err) } } -// TestConcurrencyLimit_RejectStoreAndEvent verifies that a rejected run is stored -// with terminal status "rejected" and an error message is recorded. -func TestConcurrencyLimit_RejectStoreAndEvent(t *testing.T) { - sa := newSlowAdapter() +// TestAdmissionQueue_ParentTimeoutBeatsQueueTimeout verifies that even when a +// provider queue timeout IS configured, a parent request timeout that fires +// first is still classified as concurrency_unavailable, not queue_timeout. +func TestAdmissionQueue_ParentTimeoutBeatsQueueTimeout(t *testing.T) { + // queue_timeout_ms=10s (long), but the run's request timeout is 1s, so the + // parent deadline fires first. + sa := newQueuedSlowAdapter("slow", 1, 4, 10*time.Second) router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} - nd, st := makeNodeWithConcurrency(t, router, 1) + nd, st := makeNodeWithConcurrency(t, router, 0) + + hold := make(chan error, 1) + go func() { + hold <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "pb-hold", Adapter: "slow", Target: "v1", Background: true, + }) + }() + waitStarted(t, sa, "pb-hold") + + err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "pb-timeout", Adapter: "slow", Target: "v1", TimeoutSec: 1, + }) + if !errors.Is(err, node.ErrConcurrencyLimitExceeded) { + t.Fatalf("expected ErrConcurrencyLimitExceeded, got %v", err) + } + if strings.Contains(err.Error(), "queue timeout") { + t.Fatalf("parent deadline (earlier than queue timeout) must not be queue timeout, got %q", err.Error()) + } + rec := requireStatus(t, st, "pb-timeout", "rejected") + if strings.Contains(rec.Error, "queue timeout") { + t.Fatalf("store error must not contain queue timeout, got %q", rec.Error) + } + + sa.releaseRun("pb-hold") + if err := <-hold; err != nil { + t.Fatalf("pb-hold: %v", err) + } +} + +// TestConcurrencyLimit_RejectStoreAndEvent verifies that a rejected run (queue +// overflow) is stored with terminal status "rejected" and an error message. +func TestConcurrencyLimit_RejectStoreAndEvent(t *testing.T) { + // capacity=1, max_queue=0: any second concurrent run overflows immediately. + sa := newQueuedSlowAdapter("slow", 1, 0, 0) + router := &fixedRouter{adapterName: "slow", adapters: map[string]runtime.Adapter{"slow": sa}} + nd, st := makeNodeWithConcurrency(t, router, 0) errc := make(chan error, 1) go func() { errc <- nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ - RunId: "run-hold", Adapter: "slow", Target: "v1", + RunId: "run-hold", Adapter: "slow", Target: "v1", Background: true, }) }() - select { - case <-sa.started: - case <-time.After(2 * time.Second): - t.Fatal("run-hold never started") - } + waitStarted(t, sa, "run-hold") - // Second run must be rejected. + // Second run must be rejected (queue capacity is 0). err := nd.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ RunId: "run-rejected", Adapter: "slow", Target: "v1", }) @@ -1579,7 +2042,7 @@ func TestConcurrencyLimit_RejectStoreAndEvent(t *testing.T) { t.Fatal("expected non-empty error message for rejected run") } - close(sa.release) + sa.releaseRun("run-hold") if err := <-errc; err != nil { t.Fatalf("run-hold: %v", err) } diff --git a/apps/node/internal/node/run_manager.go b/apps/node/internal/node/run_manager.go index c275b19..39af698 100644 --- a/apps/node/internal/node/run_manager.go +++ b/apps/node/internal/node/run_manager.go @@ -4,11 +4,14 @@ import ( "context" "errors" "sync" - "sync/atomic" + "time" ) -// ErrConcurrencyLimitExceeded is returned when a run is rejected because the -// active run count has reached the effective concurrency limit. +// ErrConcurrencyLimitExceeded is returned when a run is rejected because +// admission could not be granted under the current concurrency policy. It is +// kept as the wrapping sentinel for backward compatibility; the concrete reason +// (queue full, queue timeout, concurrency unavailable) is carried by +// admissionError and surfaced through admissionRejectReason. var ErrConcurrencyLimitExceeded = errors.New("concurrency limit exceeded") type runHandle struct { @@ -52,41 +55,329 @@ func (m *runManager) cancelRun(runID string) bool { return true } -// permitManager enforces a concurrency limit using an atomic counter. -// limit <= 0 means unlimited. Policy is reject-on-exceed (no queue, no wait). -type permitManager struct { - limit int32 - active atomic.Int32 +// rejectReason classifies why a run could not be admitted or had to be +// terminated before execution. +type rejectReason string + +const ( + // reasonQueueFull means the FIFO admission queue is already at max_queue. + reasonQueueFull rejectReason = "queue_full" + // reasonQueueTimeout means the request waited past queue_timeout_ms. + reasonQueueTimeout rejectReason = "queue_timeout" + // reasonConcurrencyUnavailable means admission failed because the caller's + // context was cancelled while waiting for a slot. + reasonConcurrencyUnavailable rejectReason = "concurrency_unavailable" +) + +// errAdmissionQueueTimeout is the context cancellation cause attached to the +// provider queue-timeout deadline. It lets wait() distinguish a real +// queue_timeout_ms expiry from an unrelated parent deadline (e.g. the run's +// RunRequest.timeout_sec), so only the former is reported as queue_timeout. +var errAdmissionQueueTimeout = errors.New("admission queue timeout") + +// admissionError carries the concrete rejection reason while still satisfying +// errors.Is(err, ErrConcurrencyLimitExceeded) for callers relying on the +// legacy sentinel. +type admissionError struct { + reason rejectReason + msg string } -func newPermitManager(limit int) *permitManager { - return &permitManager{limit: int32(limit)} -} +func (e *admissionError) Error() string { return e.msg } -// tryAcquire attempts to take a permit. Returns false immediately if the limit -// is already reached. limit <= 0 always succeeds. -func (p *permitManager) tryAcquire() bool { - if p.limit <= 0 { - p.active.Add(1) - return true +func (e *admissionError) Is(target error) bool { return target == ErrConcurrencyLimitExceeded } + +func newQueueFullError(maxQueue int) *admissionError { + return &admissionError{ + reason: reasonQueueFull, + msg: queueFullMessage(maxQueue), } - for { - cur := p.active.Load() - if cur >= p.limit { - return false +} + +func newQueueTimeoutError(timeout time.Duration) *admissionError { + return &admissionError{ + reason: reasonQueueTimeout, + msg: queueTimeoutMessage(timeout), + } +} + +func newConcurrencyUnavailableError() *admissionError { + return &admissionError{ + reason: reasonConcurrencyUnavailable, + msg: "concurrency unavailable: admission cancelled before a slot was free", + } +} + +func queueFullMessage(maxQueue int) string { + return "queue full (max_queue=" + itoa(maxQueue) + ")" +} + +func queueTimeoutMessage(timeout time.Duration) string { + return "queue timeout exceeded (queue_timeout=" + timeout.String() + ")" +} + +// admissionRejectReason extracts the rejectReason from an admission error, +// defaulting to concurrency_unavailable for unknown errors. +func admissionRejectReason(err error) rejectReason { + var ae *admissionError + if errors.As(err, &ae) { + return ae.reason + } + return reasonConcurrencyUnavailable +} + +// admissionMessage extracts the human-readable rejection message. +func admissionMessage(err error) string { + var ae *admissionError + if errors.As(err, &ae) { + return ae.msg + } + if err != nil { + return err.Error() + } + return string(reasonConcurrencyUnavailable) +} + +// fifoGate is a FIFO-fair counting semaphore. capacity <= 0 means unlimited +// (acquire always succeeds immediately and is never queued). maxQueue < 0 means +// an unbounded queue; maxQueue == 0 disallows queueing (a full gate rejects +// immediately); maxQueue > 0 caps the queue length. queueTimeout <= 0 means no +// timeout while waiting. +type fifoGate struct { + mu sync.Mutex + capacity int + maxQueue int + queueTimeout time.Duration + inFlight int + waiters []chan struct{} +} + +func newFifoGate(capacity, maxQueue int, queueTimeout time.Duration) *fifoGate { + return &fifoGate{ + capacity: capacity, + maxQueue: maxQueue, + queueTimeout: queueTimeout, + } +} + +// reserve takes a slot without blocking. When capacity is free it grants the +// slot immediately (granted=true, ch=nil). When full it appends a FIFO waiter +// and returns its channel (granted=false, ch!=nil) which the caller waits on via +// waitOn. When the queue is already at max_queue it returns a queue_full error. +func (g *fifoGate) reserve() (granted bool, ch chan struct{}, err error) { + g.mu.Lock() + defer g.mu.Unlock() + if g.capacity <= 0 || g.inFlight < g.capacity { + g.inFlight++ + return true, nil, nil + } + if g.maxQueue >= 0 && len(g.waiters) >= g.maxQueue { + return false, nil, newQueueFullError(g.maxQueue) + } + ch = make(chan struct{}) + g.waiters = append(g.waiters, ch) + return false, ch, nil +} + +// waitOn blocks until the reserved waiter ch is promoted to an execution slot or +// the context is cancelled (the caller derives any queue-timeout deadline onto +// ctx). On context cancellation it abandons the waiter and returns an +// admissionError; on success (including a promotion that races the cancel) it +// returns nil and the caller holds a live slot. +func (g *fifoGate) waitOn(ctx context.Context, ch chan struct{}) error { + select { + case <-ch: + // Promoted to an execution slot; inFlight was incremented by release. + return nil + case <-ctx.Done(): + if g.abandon(ch) { + return newConcurrencyUnavailableError() } - if p.active.CompareAndSwap(cur, cur+1) { + return nil + } +} + +// abandon removes ch from the waiter queue. It returns true if ch was still +// waiting (so the caller owns the failure), or false if ch had already been +// promoted (so the caller now holds a live slot it must release). +func (g *fifoGate) abandon(ch chan struct{}) bool { + g.mu.Lock() + defer g.mu.Unlock() + for i, w := range g.waiters { + if w == ch { + g.waiters = append(g.waiters[:i], g.waiters[i+1:]...) return true } } + return false } -// release returns a permit. Must be called exactly once per successful tryAcquire. -func (p *permitManager) release() { - p.active.Add(-1) +// release returns a slot. If a waiter is queued it is promoted in FIFO order, +// keeping inFlight unchanged (the slot transfers to the promoted waiter). +func (g *fifoGate) release() { + g.mu.Lock() + defer g.mu.Unlock() + if len(g.waiters) > 0 { + next := g.waiters[0] + g.waiters = g.waiters[1:] + close(next) + return + } + if g.inFlight > 0 { + g.inFlight-- + } } -// activeCount returns the number of currently held permits (for observability). -func (p *permitManager) activeCount() int { - return int(p.active.Load()) +// activeCount returns the number of in-flight (executing) slots. +func (g *fifoGate) activeCount() int { + g.mu.Lock() + defer g.mu.Unlock() + return g.inFlight +} + +// queuedCount returns the number of requests currently waiting for a slot. +func (g *fifoGate) queuedCount() int { + g.mu.Lock() + defer g.mu.Unlock() + return len(g.waiters) +} + +// admissionManager composes the node-wide global gate with a per-adapter gate. +// A run is admitted only when it holds both: the adapter gate is reserved first +// (so per-provider FIFO ordering is preserved without head-of-line blocking +// across adapters), then the global gate. +type admissionManager struct { + global *fifoGate + adapter *fifoGate +} + +// admissionTicket holds the reservation state between enqueue and wait. queued +// reports whether the request had to wait for a slot (vs. running immediately). +type admissionTicket struct { + mgr *admissionManager + adapterCh chan struct{} // non-nil if waiting on the adapter gate + globalCh chan struct{} // non-nil if waiting on the global gate + adapterHeld bool + globalHeld bool + queued bool +} + +// enqueue reserves only the per-adapter FIFO position, without blocking. It +// returns a queue_full admissionError when the adapter queue is at max_queue; +// otherwise it returns a ticket whose wait() blocks until the run is admitted. +// The global gate is intentionally NOT reserved here: reserving it before the +// adapter slot is granted would let a run still waiting for its adapter slot +// occupy a global slot, blocking a different adapter's runnable queue head +// (cross-adapter head-of-line blocking). The global slot is reserved in wait() +// only after the adapter slot is actually held. Background callers can return to +// the transport after enqueue and run wait()+execute in a goroutine. +func (a *admissionManager) enqueue() (*admissionTicket, error) { + t := &admissionTicket{mgr: a} + + granted, ch, err := a.adapter.reserve() + if err != nil { + return nil, err + } + if granted { + t.adapterHeld = true + } else { + t.adapterCh = ch + t.queued = true + } + return t, nil +} + +// wait blocks until the run holds both the adapter slot and the global slot. The +// adapter slot is awaited first; only once it is held is the global slot +// reserved and awaited, so a queued run never occupies a global slot it cannot +// yet use. The adapter gate's queue timeout (when configured) bounds the total +// admission wait across both gates via a derived deadline. On failure it unwinds +// any held slot and returns the admissionError. +func (t *admissionTicket) wait(ctx context.Context) error { + // Derive a single deadline from the adapter queue timeout so the global wait + // cannot extend total queue time past the configured policy. + if to := t.mgr.adapter.queueTimeout; to > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeoutCause(ctx, to, errAdmissionQueueTimeout) + defer cancel() + } + + if t.adapterCh != nil { + if err := t.mgr.adapter.waitOn(ctx, t.adapterCh); err != nil { + t.adapterCh = nil + t.release() + return t.mapDeadline(ctx, err) + } + t.adapterCh = nil + t.adapterHeld = true + } + + if t.mgr.global != nil { + granted, ch, err := t.mgr.global.reserve() + if err != nil { + // The global gate has an unbounded queue, so reserve never returns an + // overflow error; guard defensively and unwind the adapter slot. + t.release() + return err + } + if granted { + t.globalHeld = true + } else if err := t.mgr.global.waitOn(ctx, ch); err != nil { + t.release() + return t.mapDeadline(ctx, err) + } else { + t.globalHeld = true + } + } + return nil +} + +// mapDeadline reclassifies a context-cancellation admission error as a queue +// timeout only when the cancellation cause is the provider queue-timeout +// deadline. A parent cancellation or a parent deadline (e.g. the run's +// RunRequest.timeout_sec) keeps the original concurrency_unavailable reason, so +// an unrelated request timeout is never mislabeled as queue_timeout (which would +// also wrongly emit queue_timeout=0s when queue_timeout_ms is disabled). +func (t *admissionTicket) mapDeadline(ctx context.Context, err error) error { + if errors.Is(context.Cause(ctx), errAdmissionQueueTimeout) && admissionRejectReason(err) == reasonConcurrencyUnavailable { + return newQueueTimeoutError(t.mgr.adapter.queueTimeout) + } + return err +} + +// release returns any slot the ticket holds. It is safe to call once the ticket +// is done (after a completed run) or to unwind a partial reservation on failure. +func (t *admissionTicket) release() { + if t.globalHeld && t.mgr.global != nil { + t.mgr.global.release() + t.globalHeld = false + } + if t.adapterHeld { + t.mgr.adapter.release() + t.adapterHeld = false + } +} + +// itoa is a tiny strconv.Itoa shim kept local to avoid importing strconv solely +// for reason-message formatting. +func itoa(n int) string { + if n == 0 { + return "0" + } + neg := n < 0 + if neg { + n = -n + } + var buf [20]byte + i := len(buf) + for n > 0 { + i-- + buf[i] = byte('0' + n%10) + n /= 10 + } + if neg { + i-- + buf[i] = '-' + } + return string(buf[i:]) } diff --git a/apps/node/internal/store/store.go b/apps/node/internal/store/store.go index 3df8249..5f2aaac 100644 --- a/apps/node/internal/store/store.go +++ b/apps/node/internal/store/store.go @@ -148,6 +148,16 @@ func (s *Store) CompleteRun(ctx context.Context, runID, status, errMsg string) e return err } +// MarkRunning promotes a run to the "running" status, used when a queued run is +// admitted into an execution slot. +func (s *Store) MarkRunning(ctx context.Context, runID string) error { + _, err := s.db.ExecContext(ctx, + `UPDATE runs SET status=? WHERE run_id=?`, + "running", runID, + ) + return err +} + // GetRun retrieves a run record by ID. func (s *Store) GetRun(ctx context.Context, runID string) (*RunRecord, error) { row := s.db.QueryRowContext(ctx,