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

[suggestion] Fully asynchronous client API #3130

Open
8 tasks
appetrosyan opened this issue Feb 9, 2023 · 2 comments
Open
8 tasks

[suggestion] Fully asynchronous client API #3130

appetrosyan opened this issue Feb 9, 2023 · 2 comments
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@appetrosyan
Copy link
Contributor

appetrosyan commented Feb 9, 2023

Feature request

I propose including a version of the API calls which is fully asynchronous and consider further extending the scope of the issue to include generic HTTP transport.

Specifically we want to ensure most implementations don't need to provide an async_client implementation (c.f. https://github.com/soramitsu/iroha2-longevity-load-rs/tree/iroha2-dev/src/async_client), and can instead specify their preferred dependencies in feature flags and rely on the provided library.

Alternatively, a feasible solution would be to fully generify the API and provide specific implementations as separate crates, e.g. iroha_client_awc and/or iroha_client_reqwest as plugins.

Motivation

#3117 shows a method of obtaining cancel-able thread-based transaction submission. However, the API chosen for the initial implementation results in a counter-intuitive API, one which most of our users are likely to ignore.

More importantly, field testing showed that the use cases in which threading would offer advantages are few and far between. Our original assumption about the workflow being mostly thread based turned out to be completely wrong as evidenced by the inclusion of an async_client in the blockchain explorer, longevity load script as well as several internal projects.

It should also be considered a performance optimisation, given that threading is likely to not fully utilise the threads given the usage pattern.

Who can help?

@pesterev @appetrosyan @0x009922

Meeting minutes for Feb. 10.

Transport agnosticism not as important as assumed, hard-coding awc has advantages. Code size impact is minimal.

  • Make iroha_client a lightweight API crate,
  • Use a separate iroha_client_awc which implements the transport logic to be used in e.g. iroha_client_cli.
  • Update iroha_client_cli to use new crate,
  • Update longevity-load-rs to use new crate,
  • (Optional) update blockchain_explorer to use new crate,
  • (Optional) consider writing using run-time agnostic libraries.
  • (Optional) consider enabling static dispatch async in trait methods.
  • In so doing, prefer being async friendly, because most workflows need async.
@appetrosyan appetrosyan added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Feb 9, 2023
@appetrosyan
Copy link
Contributor Author

appetrosyan commented Feb 9, 2023

Comment due to @0x009922 telegram, Feb 9 2023:

I think it is not very hard to make the client's API fully Future-based, although it doesn't seem trivial to achieve the most important point (imo): iroha_client should be able to use a generic HTTP-transport instead of having a hard-code dependency such as awc, reqwest etc.

The way (as I see) to achieve it is to build the client on top of trait RequestBuilder with an output type Output: Request, where trait Request has fn send(self) -> Pin<Box>. This technique for making an async trait is how async_trait crate works (maybe this crate could be used directly, though). The drawback is:

Note that using these trait methods will result in a heap allocation per-function-call. This is not a significant cost for the vast majority of applications, but should be considered when deciding whether to use this functionality in the public API of a low-level function that is expected to be called millions of times a second.

https://rust-lang.github.io/async-book/07_workarounds/05_async_in_traits.html

I think this is not an issue for iroha_client. Even in context of the Explorer which might send a lot of simultaneous requests, it wouldn't be "millions of times a second" - Iroha isn't designed to handle such a load on Torii, is it?

By the way, it would be nice if iroha_client provide some implementations for RequestBuilder built on top of awc or whatever, configured by feature flags.

@appetrosyan
Copy link
Contributor Author

Comment due to @pesterev, telegram 9 Feb 2023:

iroha_client should be able to use a generic HTTP-transport instead of having a hard-code dependency such as awc, reqwest etc.

Honestly, I didn’t understand this point. awc and reqwest are just a HTTP libraries and provide only a transport layer.
Future is a standard interface for the whole asynchronous ecosystem of Rust.

I'm concerned about using async because it forces the client to use a specific runtime like tokio. But we can consider using runtime-agnostic libraries and make our library not depend on particular runtime. It will provide a more ergonomic API for all our client library users and won't depend on tokio.

appetrosyan added a commit to appetrosyan/iroha that referenced this issue Mar 22, 2023
…parameters

Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan appetrosyan removed their assignment Apr 24, 2023
pesterev added a commit to pesterev/iroha that referenced this issue Oct 18, 2023
…sync and async clients

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 18, 2023
…ient into sync and async parts

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

2 participants