rollout: decouple oversampling-abort teardown into a pluggable agent hook#1639
rollout: decouple oversampling-abort teardown into a pluggable agent hook#1639Shi-Dong wants to merge 3 commits into
Conversation
When oversampling collects enough samples, abort() aborts the in-flight SGLang generations but leaves the Harbor agent loops running. Those loops keep issuing fresh completion requests to SGLang until they hit their own max_seq_len / timeout, wasting inference and holding containers. Flush the agent server's in-flight /run trials (agent-agnostic, so it works for mini-swe-agent which bypasses harbor's lite_llm) right after the SGLang abort so they stop hitting SGLang and release their containers. No-op unless --agent-server-url / AGENT_SERVER_URL and session_server_instance_id are set.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to flush in-flight Harbor agent-server trials when aborting SGLang rollouts. It adds a new --agent-server-url command-line argument and a corresponding flush_agent_server helper function that sends a POST request to the agent server's /flush endpoint. Feedback suggests stripping leading and trailing whitespaces from the agent server URL and the admin secret to prevent potential issues with malformed URLs or invalid authorization headers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| agent_server_url = getattr(args, "agent_server_url", None) or os.environ.get("AGENT_SERVER_URL") | ||
| instance_id = getattr(args, "session_server_instance_id", None) | ||
| if not agent_server_url or not instance_id: | ||
| return | ||
|
|
||
| headers = None | ||
| admin_secret = os.environ.get("HARBOR_ADMIN_SECRET") | ||
| if admin_secret: | ||
| headers = {"Authorization": f"Bearer {admin_secret}"} |
There was a problem hiding this comment.
Environment variables and command-line arguments (especially URLs and secrets/tokens) are highly prone to containing accidental leading or trailing whitespaces or newlines (e.g., when loaded from .env files or Kubernetes secrets).
Why this matters:
- URLs: Leading/trailing spaces can cause
httpxto raise malformed URL or connection errors. - Secrets: Trailing newlines in
HARBOR_ADMIN_SECRETwill result in an invalidAuthorizationheader, causing the Harbor agent server to reject the request with a400or401error.
Stripping these values before use ensures robust and reliable API calls.
| agent_server_url = getattr(args, "agent_server_url", None) or os.environ.get("AGENT_SERVER_URL") | |
| instance_id = getattr(args, "session_server_instance_id", None) | |
| if not agent_server_url or not instance_id: | |
| return | |
| headers = None | |
| admin_secret = os.environ.get("HARBOR_ADMIN_SECRET") | |
| if admin_secret: | |
| headers = {"Authorization": f"Bearer {admin_secret}"} | |
| agent_server_url = getattr(args, "agent_server_url", None) or os.environ.get("AGENT_SERVER_URL") | |
| instance_id = getattr(args, "session_server_instance_id", None) | |
| if not agent_server_url or not instance_id: | |
| return | |
| agent_server_url = agent_server_url.strip() | |
| headers = None | |
| admin_secret = os.environ.get("HARBOR_ADMIN_SECRET") | |
| if admin_secret: | |
| headers = {"Authorization": f"Bearer {admin_secret.strip()}"} |
Replace the hard-coded Harbor /flush in core sglang_rollout.abort() with a generic, optional hook: it looks for a sibling abort() in the module named by --custom-agent-function-path and calls it if present. Core Miles no longer assumes the agent host speaks /flush; non-agentic and non-Harbor backends just drain as before. Move the Harbor /flush teardown into the swe_agent_function plugin (where the agent-server URL and /run already live) as its abort() hook. Drop the core --agent-server-url arg added earlier.
…ut) path Async training (train_async.py + MILES_EXPERIMENTAL_ROLLOUT_REFACTOR=1) uses InferenceRolloutFn, whose oversampling abort lives in inference_rollout_train.abort, a separate implementation from sglang_rollout.abort. The previous commit only wired the legacy sync path, so async agentic runs still leaked in-flight trials. Hoist the hook resolver into miles.utils.misc.call_agent_abort_hook (shared, no new cross-module dep) and call it from both abort paths.
Problem
With oversampling on, once the trainer has collected enough samples, the rollout aborts the in-flight SGLang generations via
/abort_request {"abort_all": true}. But the external agent loops driving the agentic rollouts are never told.Each aborted generation just looks like one finished LLM call to the agent, so the agent immediately issues the next completion request back to SGLang. The loops keep hammering SGLang and holding their sandbox containers until they independently hit their own
max_seq_len/ trial timeout — wasting inference during the drain and pinning containers.Why not a hard-coded flush in core
Miles' core rollout is agent-host-agnostic:
agentic_tool_call.generatedelegates all agent-host knowledge to a plugin loaded from--custom-agent-function-path. Baking a Harbor-specific/flushcall into the core rollout would wrongly assume every deployment uses a Harbor agent server that speaks/flush, which isn't always true.Fix (pluggable hook, both sync and async paths)
miles.utils.misc.call_agent_abort_hook(args)looks for a siblingabortcallable in the same module as the configured--custom-agent-function-pathand awaits it if present. No hook → drain as before. Zero Harbor strings, no new core args. Non-agentic and non-Harbor runs are unaffected.miles/rollout/sglang_rollout.py::abort— the legacy/sync path (default whenMILES_EXPERIMENTAL_ROLLOUT_REFACTORis unset).miles/rollout/inference_rollout/inference_rollout_train.py::abort— the experimental-refactor path used by async training (train_async.py+MILES_EXPERIMENTAL_ROLLOUT_REFACTOR=1→InferenceRolloutFn). This is the path Shi's agentic async runs actually take, so covering it is the point./flushlogic is inexamples/experimental/swe-agent-v2/swe_agent_function.pyas itsabort(args)hook — next to theAGENT_SERVER_URL+/runcode it already owns. It flushes the agent server's in-flight/runtrials for this run'ssession_server_instance_id(sendingHARBOR_ADMIN_SECRETas a bearer token when present). No-op unless the URL + instance id are available.No Harbor server change is needed —
/flushalready exists inharbor-miles-v0.13.1and returns HTTP 200 on cancel.Test plan
Flushed agent server ...and Harbor/runtasks terminate instead of continuing to call SGLang.train_async.py+ refactor): same behavior viainference_rollout_train.abort.abort:call_agent_abort_hookis a silent no-op; drain unchanged.custom_agent_function_pathis None → hook lookup short-circuits; abort behavior unchanged.