Skip to content

We dont actually need .worker_threads in search runtime#8756

Merged
generall merged 1 commit into
devfrom
search-worker-threads
Apr 22, 2026
Merged

We dont actually need .worker_threads in search runtime#8756
generall merged 1 commit into
devfrom
search-worker-threads

Conversation

@generall
Copy link
Copy Markdown
Member

No description provided.

@generall generall requested a review from timvisee April 21, 2026 15:48
coderabbitai[bot]

This comment was marked as resolved.

@qdrant qdrant deleted a comment from coderabbitai Bot Apr 22, 2026
Comment thread src/common/helpers.rs
let num_threads = common::defaults::search_thread_count(max_search_threads);
runtime::Builder::new_multi_thread()
.worker_threads(num_threads)
.worker_threads(2.min(num_threads)) // 2 because of paranoia margin
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion we should set this to zero, but it appears that Tokio doesn't allow that and panics.

I therefore vote to fully replace this Tokio runtime with a threadpool that is sync-only. That way we prevent accidental misuse and eliminate potential foot guns. We can do that in a separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some things to try with semafors, I am not sure we want something like Rayon here, as we don't want to pre-allocate all threads.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, a proper dynamic thread pool that fits our use case 👍

@generall generall merged commit 35ee534 into dev Apr 22, 2026
20 of 21 checks passed
@generall generall deleted the search-worker-threads branch April 22, 2026 08:41
@timvisee timvisee mentioned this pull request May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants