fix(core): count execution capacity in permits so a hung job cannot kill the scheduler#63
Merged
Merged
Conversation
…ill the scheduler Two defects that ARCHITECTURE_NOTES.md recorded but left unfixed, both reproduced first. A timed-out sync job permanently consumed a worker. Python cannot safely kill a thread, so a timeout marks the job failed and abandons its thread — but on a ThreadPoolExecutor that thread keeps its worker slot forever, and the pool has no way to replace it. With max_workers=2 and two hung jobs, the next job ran zero times: the node had silently stopped executing anything at all. (PR #61 fixed the queue slot; the pool slot was still gone.) And max_workers never bounded async jobs at all — they run on the event loop, not the pool. With max_workers=1, five async jobs ran concurrently. The parameter said one thing and the scheduler did another. Both come from counting capacity in threads. Capacity is now counted in permits (WorkerPool): one permit per running job, whether it runs on a worker thread or on the event loop, so max_workers finally means "jobs running at once". A job that cannot get a permit is not claimed — it stays SCHEDULED for the next poll or a node with room, rather than stranding in RUNNING until its liveness lease expires. And a timed-out job hands its permit back immediately, even though its thread lives on. The thread is still leaked; nothing can change that. But it is now counted and logged (get_worker_status()["abandoned_threads"]), so a job that hangs past its timeout shows up as a number that climbs rather than as a scheduler that mysteriously stalls. Also gives the async event loop an executor we own and name. Post-execution storage writes were pushed off the loop with run_in_executor(None, ...), which silently spins up asyncio's default pool — a fifth thread pool that nothing in Chronis configured or shut down. Timeout semantics differ by function colour and now say so on `timeout` / `timeout_seconds`: an async job is genuinely cancelled, a sync job is marked failed and abandoned. Both raise JobTimeoutError and take the same retry / on_failure path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
enginerd-kr
added
risk-high
Found while measuring what happens as abandoned threads pile up: once thread creation starts failing, every job that fails to start silently skips the run it was claimed for. The claim has already advanced next_run_time to the *next* occurrence, so rolling back only the status leaves the job SCHEDULED with a run time in the future — invisible to the poller and to misfire detection, which only looks at run times in the past. An interval job with a 300s period simply does not run for another 300 seconds, with nothing to say it was dropped. Rollback now restores the scheduled time the claim consumed, which ClaimedJob has been carrying since the claim boundary was typed. The job stays due and the next poll runs it. Also records what abandoned threads actually cost, since the question has a sharp answer: a thread blocked on I/O is ~37 KB and releases the GIL (200 of them cost 1.25x on a Python workload — noise), while a thread spinning in Python holds the GIL and is ruinous (20 of them cost 17.6x). Reaching the OS thread limit is survivable: creation fails, the job rolls back intact, and the scheduler resumes when threads free up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
추가: 좀비 스레드 누적의 실제 비용 (측정) + 그 과정에서 발견한 버그"버려진 스레드가 계속 쌓이면 어떻게 되는가"를 실제로 측정했습니다. 좀비가 무엇에 걸려 있느냐에 따라 완전히 다릅니다.
종착지(OS 스레드 한계, 이 머신 4000+)에 닿아도 크래시하지 않습니다: 스레드 생성 실패 → job은 SCHEDULED로 롤백, permit 누수 없음, 스레드가 풀리면 재개. 확인 완료. 결론: 블록형 누수는 치명적이 되기 한참 전에 눈에 보이고, CPU형 누수는 애초에 살아남을 수 없지만 카운터가 범인을 가장 빨리 지목합니다. 그 과정에서 기존 버그 발견 (
|
github-actions
Bot
removed
risk-high
Answers "what would replace the thread model?" with measurements instead of instinct. The residual problems split into two axes: scheduler machinery (an asyncio-native core would collapse the loops and timers, but buys no safety) and job isolation (only a process makes timeout a real guarantee — terminate() kills even a CPU-spinning child in ~1ms, the case that costs the thread model 17.6x in GIL contention). The recorded path, when picked up: per-job opt-in process isolation on the permit model (config(isolation="process")). WorkerPool already decoupled capacity from threads, so a ProcessSlot is an extension rather than a rewrite. Constraints noted: 1.5-25ms dispatch per process, and function resolution needs a worker initializer (lambda registrations cannot cross a process boundary) — while args/kwargs already round-trip as JSON by the storage contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
enginerd-kr
added
risk-high
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
요약
docs/ARCHITECTURE_NOTES.md가 "아직 고치지 않은 약점"으로 기록해 둔 항목 중 운영 위험이 가장 큰 동시성 모델을 해결합니다. 두 결함 모두 수정 전에 재현했고, 회귀 테스트는 RED 검증을 거쳤습니다.결함 1 — 타임아웃된 sync job이 스케줄러를 죽인다
파이썬은 스레드를 안전하게 죽일 수 없으니 타임아웃은 스레드를 버립니다. 그런데
ThreadPoolExecutor에서 버려진 스레드는 워커 슬롯을 영구 점유하고, 풀은 그 워커를 대체할 방법이 없습니다.max_workers개가 행에 걸리면 그 노드는 조용히 아무것도 실행하지 않게 됩니다.결함 2 —
max_workers가 async job을 전혀 제한하지 않는다async job은 풀이 아니라 이벤트 루프에서 돕니다. 파라미터가 말하는 것과 스케줄러가 하는 일이 달랐습니다.
해결: 용량의 단위를 "스레드"에서 "permit"으로
두 결함의 뿌리는 같습니다 — 용량을 스레드로 세고 있었던 것.
WorkerPool이 permit으로 셉니다:max_workers는 "동시에 실행 중인 job 수"라는 뜻을 실제로 갖습니다.정직한 한계
버려진 OS 스레드는 여전히 남습니다. 이건 바꿀 수 없습니다. 대신 이제 집계되고 로깅됩니다 —
get_worker_status()["abandoned_threads"]. 타임아웃을 한참 넘겨 매달린 job은 "숫자가 계속 올라가는 것"으로 드러나지, "스케줄러가 이유 없이 멈추는 것"으로 나타나지 않습니다. 그 누수의 원인은 스케줄러가 아니라 job에 있고, 이제 그걸 볼 수 있습니다.검증:
max_workers=4에 타임아웃 job 12개 투입 →abandoned_threads: 12,available: 4(용량 완전 회복), 실제 스레드 수는 상한(max_workers + abandoned) 내.덤: 이름 없던 5번째 스레드 풀
async job의 후처리(스토리지 쓰기)가
run_in_executor(None, ...)로 asyncio의 기본 executor에 흘러가고 있었습니다 — Chronis가 설정하지도, 종료하지도 않는 풀. 이제 명시적으로 소유하고 이름(chronis-async-io-)을 붙입니다.타임아웃 시맨틱 문서화
같은
timeout파라미터인데 함수 색깔에 따라 동작이 달랐고, 어디에도 적혀 있지 않았습니다. 이제 명시합니다: async는 실제로 취소되고(asyncio.wait_for), sync는 실패로 표시된 뒤 방치됩니다(파이썬 한계). 둘 다JobTimeoutError→ 동일한 재시도/on_failure경로.트레이드오프
스레드 재사용이 사라집니다(job당 스레드 생성 ~50µs). Chronis의 케이던스(폴링 ≥0.1s, "human-scale timing" 표방)에서 800 job/s여도 CPU 4% 수준이라 수용 가능하다고 판단했습니다. 대신 동시 스레드 수에 상한이 생깁니다 — 기존엔 async가 무제한이었습니다.
테스트
WorkerPool단위 테스트 12개(reserve/release/abandon 멱등성, 포화, shutdown이 좀비에 안 걸림), 통합 회귀 4개docs/ARCHITECTURE_NOTES.md: 해당 항목을 "동작 방식"으로 옮기고 남은 한계를 기록남은 로드맵 (문서 유지)
misfire 정책 3개 모듈 산재 + 무제한
run_all, producer 전용JobClient부재, 닫힌 트리거 확장.🤖 Generated with Claude Code