[WIP][perf][tinker] Cut database work off the multi-LoRA critical path

[avigyabb]

Superseded — being split up. This PR is now a reference for the whole investigation; it will not be merged as-is. The pieces are going out separately:

• [perf][tinker] Scan pending requests without loading their payloads #1976 — engine scheduling scan (the largest win; parked-backlog scan 249ms -> 1.2ms)
• next — retry the result write so a forwarded sample cannot hang forever
• then — shared batched poller for retrieve_future
• then — orjson codec for JSON payload columns
• then — DB path handling (node-local default, network-filesystem warning)

What

Reduces the database work on the Tinker server's hot paths, which showed up as poor performance under multi-LoRA training. Three commits:

1. Improve SQLite DB path support — default the DB to node-local /tmp/skyrl_tinker/tinker.db, create missing parent dirs, warn when the SQLite file sits on a network/FUSE filesystem (where locking and WAL are unreliable and produce database is locked), and pair WAL with synchronous=NORMAL.
2. Cut DB work off the multi-LoRA critical path — the measured fixes below.
3. Retry the result write so a forwarded sample cannot hang forever — a latent permanent-wedge bug found while investigating the stuck Tinker Fully Async E2E CI job; see "The hang, not just the slowness" below.

How this was measured

New skyrl/benchmarks/bench_tinker_db.py: no GPU, no HTTP server, no inference engine. It drives the same functions the API server and engine use, and reports SQL statement and commit counts alongside timings, so the numbers move when those code paths change. It also tolerates an engine without scan_pending_requests, so it can be run against an older checkout for a direct before/after.

uv run --isolated --extra dev --extra tinker python skyrl/benchmarks/bench_tinker_db.py \
    --num-models 16 --concurrency 128 --num-requests 512 --blocked-requests 512 \
    --await-waiters 512 --poller shared

What was actually slow

The reported symptom (serialized writes) was real, but the dominant costs were elsewhere:

• The engine's scheduling scan loaded every pending payload on every poll. find_batchable_* / find_single_requests each SELECTed full FutureDB rows for all pending requests, then discarded the ones parked behind a barrier — which is exactly the multi-LoRA steady state (some adapters stepping while others queue). With a 512-request parked backlog that was 534 ms of wasted deserialization per 100 ms poll tick, so the engine sat CPU-saturated and was slow to dispatch.
• retrieve_future polled per caller. Each in-flight future opened its own session and ran its own SELECT on a 100 ms → 1 s backoff. At 512 concurrent rollouts that is 1038 SQL statements/sec of pure polling, plus up to a second of added latency.
• json.dumps dominated the write path, not the commits — 0.40 s of a 0.77 s / 256-request submit.

Changes

• Scheduling never touches payloads. TinkerEngine.scan_pending_requests reads only (request_id, model_id, request_type) through a new ix_futures_pending_scan covering index (verified: SEARCH futures USING COVERING INDEX, no table access). Barrier logic is now a pure function of that metadata, and request_data is fetched only for the requests actually being dispatched. One scan feeds all four finders instead of issuing eight queries. Sample batch compatibility reads checkpoint_id via a JSON path extraction rather than loading whole prompts.
• One shared batched poller. New skyrl/tinker/futures.py adds FutureWaiter: callers await an asyncio future and a single background task resolves all of them with one WHERE request_id IN (...) query per 50 ms tick. Results written by the API process itself (forwarded samples) call notify() and skip the round trip entirely. The tighter interval also lowers latency relative to the old backoff.
• Fast JSON codec for payload columns. json_engine_kwargs() installs orjson when available (~7× faster encode, ~3× faster decode on these numeric arrays; byte-compatible round-trip) with a stdlib fallback. Added to the tinker extra so the fast path isn't accidental.
• Completion writes update by primary key. The forwarding clients previously did session.get(FutureDB, request_id) — loading the row's full request payload — just to write a result. Now a targeted UPDATE.
• create_missing_indexes() backfills declared indexes on databases created before they existed, since create_all skips tables it already finds. Idempotent, safe on every startup.

Results

16 adapters, 128 concurrent submitters, 512 requests plus 512 parked behind barriers, ~76 KiB payload per request:

	before	after
parked-backlog scan (runs every poll)	534 ms	1.9 ms
dispatchable-batch scan	1906 ms	965 ms
submit throughput	257 op/s	492 op/s
submit p99 latency	1020 ms	390 ms
retrieve_future SQL load	1038 stmt/s	33 stmt/s
DB file size	92.5 MB	80.4 MB

The hang, not just the slowness

The Tinker Fully Async E2E CI job was wedging during sample and dying on the SDK's No progress made in 7200s guard. Investigating it turned up a latent bug in the same area, reachable by exactly the contention above.

When sampling is forwarded (non-colocated backend, as that job uses), asample writes an EXTERNAL future and hands off to a fire-and-forget asyncio.create_task. That task's result write is the only thing that can ever complete the request — the engine excludes EXTERNAL from scheduling and nothing reaps pending rows — and the write sat outside the surrounding try/except. So a single failure there did not fail one request; it left the row pending forever while the client polled until its own deadline, surfacing as a hang rather than an error.

That failure is reachable: the async engine takes SQLAlchemy's defaults, so 15 connections (5 + 10 overflow) with a 30 s checkout timeout. With per-caller retrieve_future polling, checkouts scaled with in-flight requests. Reproduced at the DB layer for that job's 2048-concurrent-sample shape:

	per-caller polling	shared poller
polls demanded / achieved	2048/s → 1185/s (saturated)	1 query per tick, flat in waiter count
session_heartbeat-shaped write	p50 1084 ms, max 1682 ms	p50 2 ms, max 5 ms

That heartbeat starvation matches the CI log, where session_heartbeat was failing for 120–220 s from the first sampling wave onward. Under that much contention a completion write can exceed the 30 s checkout timeout, and one SATimeoutError orphaned the sample.

Fixes: complete_future retries SATimeoutError and OperationalError with exponential backoff (6 attempts, ~1 min) — pool-checkout timeouts and database is locked are precisely what a burst of simultaneous completions produces, so they must not be terminal; other exceptions still propagate immediately. Both forwarding clients now catch and log anything that still escapes, so an orphaned request is diagnosable in server.log instead of vanishing with the task. Verified against a genuinely saturated pool (pool_size=1, no overflow): two logged retries, then success, where previously the first timeout orphaned the row.

Caveat on causation: this reproduces the mechanism and the symptom, and the shared poller removes the contention that triggers it — but on an uncontended box I reached ~1.1 s heartbeat latency, not the 120 s+ in the CI log, and could not make the completion write fail outright. Getting there needs the 4×L4 box's contention (FSDP trainer + 2 vLLM engines + router + engine subprocess competing with the API event loop). Sampling was also already very slow before anything wedged (sampling_time_max 600 s at step 0, 759 s at step 1), consistent with the vLLM prefix-cache reset being a separate contributor. I'd treat this as removing a necessary condition, not as a proven single root cause.

Deliberately not done

• Group-committing submissions. Built and measured: only 1.18× throughput once orjson removed the JSON encoding cost (p99 did improve 3.5×). More importantly, batching the hot endpoints while save_weights-style endpoints commit directly would systematically reorder request_id allocation — which the engine's barrier scheduling depends on. Not worth that hazard; the reasoning is recorded in .claude/docs/tinker.md so it isn't re-litigated.
• Retention for completed futures. Rows are never deleted and each keeps its full request payload, so a long run grows the SQLite file without bound. Pruning user-visible results needs a policy decision on how long they must stay fetchable via retrieve_future, so it is out of scope here.

Testing

uv run --isolated --extra dev --extra jax --extra tinker pytest tests/tinker/ --ignore=tests/tinker/skyrl_train

76 passed, including the test_api.py integration tests that launch a real server subprocess and exercise retrieve_future and the engine loop end to end.

New coverage:

• tests/tinker/test_futures.py — waiter resolution, failed status, 404 for unknown ids, timeout, multiple waiters per request, notify() without a running poller, complete_future write/missing-row/payload-preservation, retry-then-succeed, raise-after-exhausting-retries, no-retry-on-programming-errors, and an assertion that query count does not scale with waiter count.
• tests/tinker/test_engine.py — metadata scan ordering, passes held back at a barrier, one-checkpoint-per-model sample batching through the JSON extraction, and a backlog larger than the bound-parameter chunk size.
• tests/tinker/test_db.py — index backfill on a pre-existing database, and a query-plan assertion that the scan stays index-only.

🤖 Generated with Claude Code