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

Only spawn_blocking for tx creation in DA queue #1321

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

yaziciahmet
Copy link
Contributor

@yaziciahmet yaziciahmet commented Oct 10, 2024

Description

It doesn't make much sense to spawn the whole DA queue in a spawn_blocking, due to the fact that it waits empty for messages most of the time, hence, it doesn't need a whole thread to itself. Instead, with this PR, queue is spawned as a tokio runtime worker, so it doesn't occupy any resources while waiting, but only acquires a thread with spawn_blocking when it's time to create the transaction and find the right prefix (or nonce).

Linked Issues

  • Fixes # (issue, if applicable)
  • Related to # (issue)

Testing

Describe how these changes were tested. If you've added new features, have you added unit tests?

Docs

Describe where this code is documented. If it changes a documented interface, have the docs been updated?

Copy link

codecov bot commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 71.92982% with 16 lines in your changes missing coverage. Please review.

Project coverage is 77.5%. Comparing base (341f241) to head (804333c).
Report is 1 commits behind head on nightly.

Files with missing lines Patch % Lines
...c/helpers/builders/light_client_proof_namespace.rs 55.5% 12 Missing ⚠️
crates/bitcoin-da/src/service.rs 66.6% 4 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...n-da/src/helpers/builders/batch_proof_namespace.rs 100.0% <100.0%> (ø)
crates/bitcoin-da/src/helpers/builders/tests.rs 100.0% <100.0%> (+1.9%) ⬆️
crates/bitcoin-da/src/service.rs 51.4% <66.6%> (-0.4%) ⬇️
...c/helpers/builders/light_client_proof_namespace.rs 26.9% <55.5%> (+21.4%) ⬆️

Copy link
Contributor

@rakanalh rakanalh left a comment

Choose a reason for hiding this comment

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

👍

@yaziciahmet yaziciahmet merged commit 403a138 into nightly Oct 10, 2024
14 checks passed
@yaziciahmet yaziciahmet deleted the yaziciahmet/improve-da-queue-spawn branch October 10, 2024 17:04
eyusufatik added a commit that referenced this pull request Oct 11, 2024
* only spawn_blocking for nonce search (#1321)

* Use spawn_blocking in bitcoin service not in tx builders (#1324)

* Use spawn_blocking in bitcoin service not in tx builders

* Clone outside of spawn

---------

Co-authored-by: yaziciahmet <[email protected]>

* fix bugs regarding tx gas limit (#1323)

* fix bugs regarding tx gas limit

* group gas limit capping functionality to a function

* prepare v0.5.4 (#1327)

* update version number in doc and update changelog

* update neglected CITREA_VERSION constant

---------

Co-authored-by: Ahmet Yazıcı <[email protected]>
Co-authored-by: Roman <[email protected]>
Co-authored-by: yaziciahmet <[email protected]>
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