Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use threadpools for STF Tasks #2159

Merged
merged 13 commits into from
Sep 27, 2023
Merged

feat: use threadpools for STF Tasks #2159

merged 13 commits into from
Sep 27, 2023

Conversation

felixfaisal
Copy link
Member

@felixfaisal felixfaisal commented Sep 25, 2023

Resolves #1958
Utilizing a threadpool offers a notable advantage over executing tasks sequentially, particularly in scenarios that are I/O-bound or involve a combination of I/O and CPU operations. One of the primary strengths of a threadpool is its ability to distribute tasks effectively. When faced with tasks of varying durations, a threadpool can evenly distribute the workload across its threads, ensuring that no single thread is disproportionately burdened.

However, this concurrency presents a challenge: when multiple tasks attempt to submit trusted calls, nonce conflicts can arise.
To address this challenge, I've implemented an MPSC channel. By routing submissions through a single receiver thread, we can maintain the asynchronicity of task completions while ensuring that trusted call submissions don't encounter nonce conflicts. By having tasks only signal their completion through these channels, rather than submitting trusted calls directly, we've effectively introduced a layer that ensures ordered submissions, resolving the nonce conflict issue.

@felixfaisal felixfaisal marked this pull request as ready for review September 26, 2023 05:09
@felixfaisal felixfaisal changed the title [WIP] feat: use threadpools for STF Tasks feat: use threadpools for STF Tasks Sep 26, 2023
Copy link
Member

@kziemianek kziemianek left a comment

Choose a reason for hiding this comment

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

Looks ok.
Would be good to have one more approval on this one.

let context_for_thread = context.clone();
std::thread::spawn(move || loop {
if let Ok((shard, hash, call)) = to_receiver.recv() {
error!("Submitting trusted call to the pool");
Copy link
Member

Choose a reason for hiding this comment

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

Why this is an error ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to info

Copy link
Member

@kziemianek kziemianek Sep 27, 2023

Choose a reason for hiding this comment

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

@felixfaisal can this be debug or trace or maybe even removed?

Imho this log entry doesn't provide any useful information and will only spawn logs.

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Looks good from my side, thanks!

I'm only thinking if we should have some tests for it - e.g. having a ThrottledRequestType where the handling logic simply sleeps for X seconds before it returns.

But that doesn't belong to the scope of this PR ofc

@felixfaisal felixfaisal merged commit 39512f3 into dev Sep 27, 2023
28 of 30 checks passed
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.

A thread pool for the STF task handler
3 participants