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

fixed tokio feature declaration #228

Closed
wants to merge 1 commit into from
Closed

Conversation

savaki
Copy link
Contributor

@savaki savaki commented Mar 28, 2022

in token 1.17.0, the feature that enables new is now called rt-multi-thread rather than rt as per documentation, https://docs.rs/tokio/latest/tokio/runtime/struct.Runtime.html

Without this change, the following errors occur on compilation:

error[E0599]: no function or associated item named `new` found for struct `tokio::runtime::Runtime` in the current scope
  --> src/sinks/aws_sqs/setup.rs:26:51
   |
26 |         let aws_config = tokio::runtime::Runtime::new()?.block_on(
   |                                                   ^^^ function or associated item not found in `tokio::runtime::Runtime`

error[E0599]: no function or associated item named `new` found for struct `tokio::runtime::Runtime` in the current scope
  --> src/sinks/aws_lambda/setup.rs:24:51
   |
24 |         let aws_config = tokio::runtime::Runtime::new()?.block_on(
   |                                                   ^^^ function or associated item not found in `tokio::runtime::Runtime`

error[E0599]: no function or associated item named `new` found for struct `tokio::runtime::Runtime` in the current scope
  --> src/sinks/aws_s3/setup.rs:52:51
   |
52 |         let aws_config = tokio::runtime::Runtime::new()?.block_on(
   |                                                   ^^^ function or associated item not found in `tokio::runtime::Runtime`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `oura` due to 3 previous errors

@savaki savaki requested a review from scarmuega as a code owner March 28, 2022 22:49
@savaki
Copy link
Contributor Author

savaki commented Mar 29, 2022

Would also like to add support for aws kinesis, but waiting on this to resolve the issue with aws sinks.

@scarmuega
Copy link
Member

hi @savaki thanks for the PR, nice catch. Since we usually build using the --all-features flag, this issue went under the radar.

Regarding the solution, do you think that we could instead use the single-threaded runtime? Something like this:

        let aws_config = tokio::runtime::Builder::new_current_thread()
            .build()?
            .block_on(
                aws_config::from_env()
                    .region(Region::new(explicit_region))
                    .load(),
            );

I believe this is possible within the scope of the rt feature flag.

Some background:

In the current design, we rely on each stage to process events sequentially, allowing us to assume that the block / tx order is preserved, which is mandatory for certain use-cases (like updating ledger-like data models). This forces us to skip the benefits of the async framework and block on each task / future sequentially. The single-threaded tokio runtime included in the rt flag seems good enough for this approach, that's why I'm hesitant on adding the multi-threaded one.

Maintaining the order of events is important in several scenarios, but I'm aware that this has a significant hit on performance. In the near future, the plan is to allow certain sinks to provide a configurable "batch mode" where events get processed concurrently, disregarding the original sequence. When we reach this point, we'll probably switch from rt to rt-multi-thread.

What do you think?

@rvcas
Copy link
Member

rvcas commented Mar 30, 2022

Oh that actually makes a lot of sense. We probably do want to preserve order I didn't think about that.

@savaki
Copy link
Contributor Author

savaki commented Mar 30, 2022

Works for me. Given the limits of protocol parameter changes, there's a good chance you may never need rt-multi-thread as data from the cardano-node can only arrive so fast once you're at tip. And full replays are so long that once you've done it once, teams are more likely to retrieve data from a cache than from the original source.

@scarmuega
Copy link
Member

@savaki I've applied the changes we discussed in #230. I'll close this issue, but feel free to re-open if required.

@scarmuega scarmuega closed this Apr 1, 2022
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