Check that Zebra is only using one version of tokio #2987
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5f88b00 to
3539dc5
Compare
7466baf to
fdbbb66
Compare
3 tasks
jvff
previously approved these changes
Nov 1, 2021
|
Nice catch! I didn't know about that file 😓 EDIT: Nevermind, just realized this is an extension of #2986. |
|
Would it make sense to merge this PR into #2986 and merge this after the Tokio upgrade? That would help to avoid feature creep in the rollup PR. IMHO, we could upgrade to Tokio first, and then fix any bugs that this PR finds afterwards. |
3 tasks
This PR tests that the dependencies are correct in the rollup PR. Duplicate crates can cause weird runtime errors and test failures. So I think it's helpful to include it in the rollup, to avoid a half-broken |
Update to latest version to add support for Tokio version 1.
It was deprecated in favor of `ServiceExt::ready`.
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.
In newer versions of Tokio `Interval` doesn't implement `Stream`, so the wrapper types from `tokio-stream` have to be used instead.
In newer versions of Tokio the `Interval` type doesn't implement `Stream`, so `tokio_stream::wrappers::IntervalStream` has to be used instead.
In newer versions of Tokio `broadcast::Receiver` doesn't implement `Stream`, so `tokio_stream::wrappers::BroadcastStream` instead. This also requires changing the error type that is used.
Newer versions of Tokio can return an error if the semaphore is closed. This shouldn't happen in `tower-batch` because the semaphore is never closed.
On newer versions of Tokio `Semaphore::acquire` can return an error if the semaphore is closed. This shouldn't happen in the test because the semaphore is never closed.
Use versions compatible with Tokio version 1.
Use a version that supports Tokio version 1.
And also update the `metrics-exporter-prometheus` to version 0.6.1. These updates are to make sure Tokio 1 is supported.
`u64` isn't supported as the histogram data type in newer versions of `metrics`.
Make it compatible with the new version of `metrics`.
Remove all constants and use the new `metrics::incement_counter!` macro.
The snapshot string isn't included in the newer version of `metrics-exporter-prometheus`.
Use a version compatible with Tokio version 1.
This seems to not be available from `sentry-tracing` anymore, so it needs to be replaced.
This seems like the replacement for `TracingIntegration`.
Suggested by a Clippy lint.
Apply all of the updates to dependencies.
Also ban git sources for tokio dependencies.
fdbbb66 to
b1e4146
Compare
jvff
approved these changes
Nov 2, 2021
|
Merged through #2994. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Duplicate tokio dependencies might result in hard-to-diagnose bugs in Zebra.
This happens because code expects a single instance of some structs, but there are multiple instances running.
If we add duplicate dependencies, Zebra compiles slower, and its binaries will be larger.
This slows down CI, and can fill up CI disks.
Scheduling
This is unexpected work in Sprint 21, to make sure that tokio is upgraded correctly.
It should be merged with the tokio roll-up PR.
Solution
deny.tomldeny.tomlReview
@jvff should review this PR.
It is based on PR #2986.
Reviewer Checklist
deny.tomlchanges makes sense