Skip to content

index: receive inserter batches off the tokio workers - #119

Open
vladwing wants to merge 1 commit into
facebookexperimental:mainfrom
vladwing:inserter-recv-off-runtime
Open

vladwing wants to merge 1 commit into
facebookexperimental:mainfrom
vladwing:inserter-recv-off-runtime

Conversation

@vladwing

Copy link
Copy Markdown
Contributor

The database inserters of the --git range, tree, --commits and lore pipelines hold a runtime worker while they wait for their next batch. They are tokio tasks that wait in a blocking std::sync::mpsc recv(), under a std Mutex all of them share. With --db-threads at or above the worker count, every worker but the one running an insert sits in recv() or in the mutex, and all of lance's async work for the run shares that one worker. On the default runtime that takes --db-threads at or above the CPU count. With the 32-worker cap in #112 it takes at most 32, so hosts with more CPUs reach it at --db-threads 32. How much slower such a run gets has not been measured.

Add recv_shared(), which runs the locked receive on tokio's blocking pool, and use it at all four sites. A waiting inserter now occupies a blocking-pool thread instead of a worker; the pool's default limit is 512 threads. recv_shared() uses spawn_blocking rather than block_in_place because block_in_place panics on the current-thread runtime that #[tokio::test] gives tests. Each site handles a poisoned receiver lock as before: the range, tree and --commits inserters panic, as their unwrap() did, and the lore inserter logs and leaves its loop. The tuple receive in git_range.rs that runs on a std thread is unchanged.

This conflicts with #113, which makes the lore inserter keep receiving until a batch holds batch_size emails. Whichever lands second resolves it by moving that loop into the closure, so the whole merge runs on the blocking pool:

recv_shared(&result_rx_clone, move |rx| {
    rx.recv().map(|mut emails| {
        while emails.len() < batch_size {
            match rx.recv() {
                Ok(more) => emails.extend(more),
                Err(_) => break,
            }
        }
        emails
    })
})

recv_shared_does_not_hold_workers parks two waiters on a live channel in a two-worker runtime and checks that a newly spawned task still runs. With the waiters calling lock().unwrap().recv() directly, the same test fails after its 10 s timeout.

The database inserters of the `--git` range, tree, `--commits` and lore
pipelines hold a runtime worker while they wait for their next batch.
They are tokio tasks that wait in a blocking `std::sync::mpsc` `recv()`,
under a std `Mutex` all of them share. With `--db-threads` at or above
the worker count, every worker but the one running an insert sits in
`recv()` or in the mutex, and all of lance's async work for the run
shares that one worker. On the default runtime that takes `--db-threads`
at or above the CPU count. With the 32-worker cap in facebookexperimental#112 it takes at
most 32, so hosts with more CPUs reach it at `--db-threads 32`. How much
slower such a run gets has not been measured.

Add `recv_shared()`, which runs the locked receive on tokio's blocking
pool, and use it at all four sites. A waiting inserter now occupies a
blocking-pool thread instead of a worker; the pool's default limit is
512 threads. `recv_shared()` uses `spawn_blocking` rather than
`block_in_place` because `block_in_place` panics on the current-thread
runtime that `#[tokio::test]` gives tests. Each site handles a poisoned
receiver lock as before: the range, tree and `--commits` inserters
panic, as their `unwrap()` did, and the lore inserter logs and leaves
its loop. The tuple receive in `git_range.rs` that runs on a std thread
is unchanged.

This conflicts with facebookexperimental#113, which makes the lore inserter keep receiving
until a batch holds `batch_size` emails. Whichever lands second resolves
it by moving that loop into the closure, so the whole merge runs on the
blocking pool:

    recv_shared(&result_rx_clone, move |rx| {
        rx.recv().map(|mut emails| {
            while emails.len() < batch_size {
                match rx.recv() {
                    Ok(more) => emails.extend(more),
                    Err(_) => break,
                }
            }
            emails
        })
    })

`recv_shared_does_not_hold_workers` parks two waiters on a live channel
in a two-worker runtime and checks that a newly spawned task still runs.
With the waiters calling `lock().unwrap().recv()` directly, the same
test fails after its 10 s timeout.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 11, 2026
vladwing added a commit to vladwing/semcode that referenced this pull request Nov 11, 2026
#[tokio::main] starts one worker per CPU, and the lore pipeline started
one email parser thread per CPU.  On a 316-CPU host that is over 600
threads for a job limited by two database inserters: the parsers sat
blocked on the result channel, and the process spent most of its CPU
time in the kernel, in futex and spinlock contention from idle tokio
workers parking and waking.

Build the runtime by hand with min(num_cpus, 32) workers, still letting
TOKIO_WORKER_THREADS override it as it does for the default runtime,
and cap the lore parser threads the same way.  The rayon pool is left
alone; -j already sizes it.  Machines with 32 CPUs or fewer see no
change.

The other modes do their CPU work on rayon, std threads or
spawn_blocking, so the tokio cap does not touch them.  The exception
is lance's FTS index build, which tokenizes on tokio workers: a full
lore FTS (re)build on a large host is now limited to 32 workers, and
TOKIO_WORKER_THREADS lifts that.

The database inserters are another matter.  The inserter tasks of the
--git, tree, --commits and lore pipelines are tokio tasks that block
their worker in a std channel recv() while they wait for a batch, so
with --db-threads at or above the worker count they can occupy every
worker and starve the async work the inserters wait on.  That was
already reachable at one worker per CPU on small hosts; with the cap it
is reachable at 32 inserters on any host.  The defect predates this
change, and facebookexperimental#119 moves those receives off the workers.
@vladwing vladwing closed this Nov 11, 2026
@vladwing vladwing reopened this Nov 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant