[perf][tinker] Add a covering index for the engine's pending scan

[avigyabb]

Stacked on #1976 — read only the second commit. That PR makes the engine's scheduling scan read only
(request_id, model_id, request_type) instead of whole rows; this one adds an index covering exactly those columns.

GitHub shows the cumulative diff (both commits) because a cross-repo PR cannot use a fork branch as its base.
The change under review here is eac1387 only — 79 lines
across 3 files. The rest belongs to #1976. Once that merges I will rebase and this will show the 79 lines alone.

What

Adds ix_futures_pending_scan on (status, request_id, request_type, model_id), so the engine's per-poll scan is answered from the index without visiting the table rows.

With only the plain status index, SQLite locates the pending rows through the index but must then visit each one to read model_id and request_type. Those rows carry request_data payloads of ~76 KiB, so they are spread far apart on disk and a few hundred row visits means a few hundred scattered page reads. An index holding all four columns the scan reads answers it outright:

EXPLAIN QUERY PLAN
  SELECT request_id, model_id, request_type FROM futures WHERE status='pending' ORDER BY request_id
→ SEARCH futures USING COVERING INDEX ix_futures_pending_scan

status is the leading column, so it supersedes the plain status index — that one is dropped rather than maintained alongside.

create_missing_indexes is here because SQLModel.metadata.create_all skips tables it already finds, so an index declared later never gets built on an existing database. Without it this change would help fresh databases and silently do nothing for every existing deployment, which is the worst kind of no-op — the code works and just keeps the slow plan. It is idempotent (checkfirst=True) and runs on every startup.

What it is actually worth

Being upfront, because it is much less than #1976 and it is not free.

Measured, isolated scan of a 512-row pending backlog with realistic payloads:

	metadata scan (warm)
plain status index	1.9 ms
covering index	0.9 ms

End-to-end, it is below noise. The parked-backlog scan in bench_tinker_db.py reads 1.2 ms with or without this index, because #1976 already removed the expensive part. The split across the two PRs is very lopsided:

• full-row query (before either PR) → 247 ms
• 3-column query, plain status index ([perf][tinker] Scan pending requests without loading their payloads #1976 alone) → 1.2 ms
• 3-column query, covering index (this PR) → 1.2 ms

So: accept this as a scan refinement and a hedge against deeper queues, not as the fix for anything.

The scaling argument, and why I am not quoting a number for it. The covering index should pull further ahead as the backlog deepens, since the plain index's cost comes from row visits that get more scattered as payloads grow. Measuring that at 2000 and 6000 pending rows gave between 1.5x and 6.5x, but the results were non-monotonic and in one case reported the warm run slower than the cold one — at hundreds of MB of payload the measurement is dominated by OS page-cache behaviour rather than by SQLite. Directionally the covering index was faster in every configuration I tried; I cannot honestly put a figure on how much.

What it costs

• Index storage: 18.1 → 51.5 bytes/row (~2.8x). Measured with dbstat over 200k rows — 10.3 MB against 3.6 MB for the status index it replaces, so roughly +33 MB per million requests. Modest next to the payloads, and next to the fact that completed futures are never pruned at all.
• Writes: unaffected. Inserts measure 59.1 → 59.4 ms for 512 rows in one transaction, and 91.2 → 89.6 ms for 128 rows in a transaction each. Status write-back measures ~105 ms either way, across both index shapes and both light and heavy scan history. I initially assumed the wider index would cost something on every status update and only found otherwise by measuring it.

Testing

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

Two new tests in tests/tinker/test_db.py:

• test_create_missing_indexes_backfills_existing_database — builds a schema without the index (as a pre-existing deployment would have), asserts the index is absent, runs the backfill, asserts it appears, and runs it again to confirm it is safe on every startup.
• test_pending_scan_uses_covering_index — asserts the query plan actually says COVERING INDEX. Without this, a later column addition to the scan could quietly turn it back into row visits with no test failing.

Unrelated flake to be aware of. Running the suite twice in a row fails test_api.py::test_training_workflow and ::test_delete_checkpoint with 404s. That is because the default database_url is a persistent file in the source tree (skyrl/tinker/tinker.db), so consecutive runs contaminate each other; deleting it first makes them pass. Pre-existing on main and unrelated to this change, but it cost me a confused half hour, so: rm -f skyrl/tinker/tinker.db* before a rerun. A separate PR in this series moves that default to node-local /tmp, though per-run isolation would be the real fix.

🤖 Generated with Claude Code