verl tool adapter: synchronous env execution blocks the agent-loop event loop
## Problem
`OumiVerlTool.execute` is `async`, but the work it does is synchronous:
```python
router = get_or_build_router(agent_data, _parent_router(self._env_config_path))
result = router.route_batch([(self._tool_id, args)])[0]
```
`route_batch` → `ExecutableEnvironment.step` → the executor runs inline on verl's asyncio worker. verl schedules many trajectories (and parallel tool calls within a turn) on one event loop, so a single slow tool call stalls every rollout sharing that worker. The first call of a rollout is worse: it also builds the environment (for a `schema_sql` database env, that materializes a snapshot and runs DDL + seed).
Not a problem for the current NL2SQL consumer — SQLite queries against a staged Spider DB are milliseconds. It matters for any environment whose `step` does slow I/O: an HTTP/MCP-backed tool, a browser env, a large seed script.
## Why `asyncio.to_thread` is not the fix
`DatabaseSession` opens its connection without `check_same_thread=False`:
```python
self.connection = sqlite3.connect(self._path, isolation_level=None)
```
so the connection is pinned to the thread that created it. A default thread-pool offload can land subsequent calls on a different worker thread and raise `ProgrammingError`. The rollback-on-close transaction semantics also assume one owning thread.
## Sketch of a real fix
Give each rollout router a dedicated single thread that both **builds** and **drives** its environments, and await it from `execute`:
- one `ThreadPoolExecutor(max_workers=1)` per rollout router, created alongside the router in `env_provider`
- `for_sample(...)` and every `route_batch(...)` for that rollout run on that thread — preserving per-router serialization and thread affinity
- shut the executor down in `_teardown` next to `router.close()`
Per-router (not global) keeps one rollout's slow call from blocking another's, which is the actual goal.
## Validation this needs
A cluster GRPO run to confirm throughput improves (or at least doesn't regress) and that DB isolation still holds — the current synchronous path is validated end to end (Spider val EX 0.367 → 0.684).
## Context
Raised by a Codex review of #2560; deliberately deferred out of that PR to keep it reviewable.
关闭于 20 天前 0 条评论