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

Update Tokio to version 1.12.0 #2920

Closed
wants to merge 2 commits into from

Conversation

jvff
Copy link
Contributor

@jvff jvff commented Oct 20, 2021

Motivation

This is part of the update to use Tokio version 1 (#2200), and must be merged together with the other PRs.

Newer versions of Tokio have some interesting features, as well as unblocking Zebra from updating other dependencies.

Solution

The Tokio version was updated to 1.12.0. This unfortunately breaks the Zebra build, so this PR can't be merged until other PRs that fix the issues are approved.

One of the fixes is to not use the StreamExt trait that was available in Tokio 0.3.6 but isn't available in Tokio 1.12.0. Some usages of that trait need some work, but one of the usages just needs to change the import so it was included in this PR.

Review

Anyone from the team can review this, but it shouldn't be merged yet.

Testing

In the tokio rollup PR, make sure that Zebra's dependency tree only has the upgraded tokio version. Also check for duplicates for other dependencies we upgraded.

If we have multiple versions, we'll be running multiple executors, and might have other subtle bugs.

We can check for duplicates using cargo deny check bans:
https://embarkstudios.github.io/cargo-deny/checks/bans/index.html#use-case---duplicate-version-detection

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

@jvff jvff requested a review from a team October 20, 2021 20:58
@jvff jvff self-assigned this Oct 20, 2021
@dconnolly dconnolly mentioned this pull request Oct 20, 2021
3 tasks
@dconnolly dconnolly self-requested a review October 25, 2021 21:43
@teor2345
Copy link
Contributor

I added a testing section:

In the tokio rollup PR, make sure that Zebra's dependency tree only has the upgraded tokio version. Also check for duplicates for other dependencies we upgraded.

If we have multiple versions, we'll be running multiple executors, and might have other subtle bugs.

We can check for duplicates using cargo deny check bans:
https://embarkstudios.github.io/cargo-deny/checks/bans/index.html#use-case---duplicate-version-detection

@mpguerra mpguerra mentioned this pull request Oct 29, 2021
10 tasks
@teor2345
Copy link
Contributor

teor2345 commented Nov 1, 2021

I added a testing section:

In the tokio rollup PR, make sure that Zebra's dependency tree only has the upgraded tokio version. Also check for duplicates for other dependencies we upgraded.

This is covered by PRs #2986 and #2987.

@teor2345
Copy link
Contributor

teor2345 commented Nov 1, 2021

Note: the tokio::stream::StreamExt import in tower-batch has been removed in the latest main branch.

jvff added 2 commits November 2, 2021 00:22
This will break the build because the code isn't ready for the update,
but future commits will fix the issues.
Use `futures::stream::StreamExt` instead, because newer versions of
Tokio don't have the `stream` feature.
@jvff jvff force-pushed the update-to-tokio-1.12.0 branch from becd6ca to 4fc5531 Compare November 2, 2021 00:23
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

lgtm!

@jvff
Copy link
Contributor Author

jvff commented Nov 2, 2021

Merged through #2994.

@jvff jvff closed this Nov 2, 2021
@jvff jvff deleted the update-to-tokio-1.12.0 branch November 24, 2021 13:00
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