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

Light client improvements #1458

Merged
merged 1 commit into from
May 14, 2024
Merged

Light client improvements #1458

merged 1 commit into from
May 14, 2024

Conversation

jbearer
Copy link
Member

@jbearer jbearer commented May 14, 2024

  • Use spawn_blocking for potentially expensive synchronous calls
  • Sleep for a shorter interval if an attempt to sync state fails
  • Configure relay server with total stake, let it compute the threshold

* Use `spawn_blocking` for potentially expensive synchronous calls
* Sleep for a shorter interval if an attempt to sync state fails
* Configure relay server with total stake, let it compute the threshold
Copy link
Contributor

@alxiong alxiong left a comment

Choose a reason for hiding this comment

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

so spawn()-ed thread in async_std's management would be killed half-way if it's long standing?

@mrain
Copy link
Contributor

mrain commented May 14, 2024

Good idea of configuring a total stake rather than a threshold. It looks cleaner.

I thought about fetching the total stake from orchestrator but I don't like adding one more dependency here.

@jbearer jbearer merged commit 90d720a into main May 14, 2024
15 checks passed
@jbearer jbearer deleted the jb/prover branch May 14, 2024 12:50
@jbearer
Copy link
Member Author

jbearer commented May 14, 2024

so spawn()-ed thread in async_std's management would be killed half-way if it's long standing?

No, I don't think that's what happened. The issue is that spawn and block_on, as used here, were not doing anything to move the blocking SNARK computation to a separate thread. It was blocking the main thread which (I believe) was causing the healthcheck API handler to not get a chance to run, and then the container orchestrator was eventually killing the process.

To summarize:

  • spawn runs a Future on a separate task, meaning the future will complete without needing to be awaited. But the same executor is still responsible for running this future, meaning it will run on an executor thread
  • block_on runs a Future to completion on the current thread. It is similar to writing a loop that eagerly polls the future until it returns Poll::Ready. It is bad to use block_on from an already async context, because it will monopolize the current thread and not allow other futures to run on it until the current one completes. In other words block_on turns an async operation into a blocking one.
  • spawn_blocking runs a blocking function on a separate thread -- one which is not part of the async executor thread pool. Rather than blocking the current thread, it yields, allowing other async tasks to run, until the blocking operation completes on the other thread. In other words, spawn_blocking turns a blocking operation into an async one

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.

3 participants