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

chore: Fully upgrade to tokio 1.x #5872

Merged
merged 120 commits into from
Apr 2, 2021
Merged

chore: Fully upgrade to tokio 1.x #5872

merged 120 commits into from
Apr 2, 2021

Conversation

lukesteensen
Copy link
Member

@lukesteensen lukesteensen commented Jan 6, 2021

#5175

Some of the things left to do:

  • Move to the new tokio-openssl API
  • Figure out new way to do socket shutdown and keepalive
  • Work around no poll_ready on new tokio channels for Pipeline
  • General updates and cleanup
  • Fix tests after upgrade

And some dependencies we're still waiting on upgrades from:

  • warp
  • rusoto
  • rdkafka
  • bollard
  • mongodb (didn't seem to affect the build, but should be upgraded in the future)

This is not a complete list, but covers most of the big ones I've run into so far.

Some of the things left to do:

- [ ] Move to the new `tokio-openssl` API
- [ ] Figure out new way to do socket shutdown and keepalive
- [ ] Work around no `poll_ready` on new tokio channels for `Pipeline`
- [ ] General updates and cleanup

And some dependencies we're still waiting on upgrades from:

- [ ] `warp`
- [ ] `rusoto`
- [ ] `rdkafka`
- [ ] `bollard`
- [ ] `mongodb`

This is not a complete list, but covers most of the big ones I've run
into so far.

Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
@fanatid fanatid mentioned this pull request Jan 12, 2021
28 tasks
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
@lukesteensen
Copy link
Member Author

lukesteensen commented Jan 20, 2021

A handful of dependencies released tokio 1.0 upgrades of their own, so I went ahead and bumped those. It appears the remaining few are:

  • bollard: seems blocked on a named pipes PR in tokio
  • mongodb: upgrade PR is open but they're still tracking down issues
  • pulsar: upgrade PR is open
  • async-graphql-warp: upgrade is merged but not yet released

The other issues we may end up semi-blocked on are the socket keepalive and shutdown APIs that have changed.

  • keepalive: Was removed but seems to be on the path to being readded as mio migrates to use socket2 socket types and tokio follows. That could take a while but it seems that we could potentially use the new SocketRef API to work around it.
  • shutdown: The API was changed not to take the half of the socket you wish to shutdown. I haven't looked in depth and it's possible that it will stick work fine for all our use cases. If not, we can probably go through SocketRef again but we'll need to make sure it's safe for an async context.

Aside from these, there is likely still a decent amount of normal upgrade work within our own code. Most seems relatively routine, but one potentially larger piece is the transition away from the Sink API for Pipeline. We could avoid or delay it by using channels from the futures crates instead of tokio, but it is something we've thought about doing anyway (#5107) and should be pretty straightforward.

Signed-off-by: Luke Steensen <luke.steensen@gmail.com>
@Geal
Copy link

Geal commented Feb 15, 2021

FYI a 2.0 of pulsar-rs with support for tokio 1.0 is imminent. We're doing a final pass of tests, everything looks good for now

@binarylogic binarylogic added this to the 2021-02-22 - Scottish Deerhound milestone Feb 22, 2021
@binarylogic
Copy link
Contributor

@pablosichert assigning this to you to take it to the finish line.

@binarylogic binarylogic removed this from the 2021-02-22 - Scottish Deerhound milestone Feb 23, 2021
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
@Geal
Copy link

Geal commented Mar 5, 2021

pulsar 2.0 with tokio 1.0 support is released

Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Copy link
Member

@bruceg bruceg left a comment

Choose a reason for hiding this comment

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

Phew, a lot of work to review here. LGTM from what I can tell, with a few minor (non-blocking) suggestions.

lib/file-source/src/file_server.rs Outdated Show resolved Hide resolved
src/tls/incoming.rs Show resolved Hide resolved
src/udp.rs Outdated Show resolved Hide resolved
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Signed-off-by: Pablo Sichert <mail@pablosichert.com>
Cargo.toml Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
lib/file-source/Cargo.toml Show resolved Hide resolved
lib/k8s-e2e-tests/Cargo.toml Show resolved Hide resolved
lib/k8s-test-framework/Cargo.toml Show resolved Hide resolved
lib/vector-api-client/Cargo.toml Show resolved Hide resolved
Copy link
Member Author

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

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

✅ You are a hero, @pablosichert!

(I can't approve because I opened the PR)

@@ -162,7 +164,7 @@ derivative = "2.1.1"
dirs-next = { version = "2.0.0", optional = true }
dyn-clone = "1.0.3"
encoding_rs = { version = "0.8", features = ["serde"] }
evmap = { version = "10.0.2", features = ["bytes"], optional = true }
evmap = { git = "https://github.com/lukesteensen/evmap.git", rev = "45ba973c22715a68c5e99efad4b072421f7ad40b", features = ["bytes"], optional = true }
Copy link
Member Author

Choose a reason for hiding this comment

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

For the record, this was only for this so the difference from a released version is minimal.

I believe the upstream crate has reorganized a bit for its next major version, but I'll take a look at getting us off of this soon.

@pablosichert
Copy link
Contributor

...
metrics_off/topology/tcp_socket/01_writer                                                            31.655 ms  -1.3335%     30.127 MiB/s    +1.3515%           0.00  none
metrics_off/topology/tcp_socket/02_writers                                                           63.421 ms  -1.9942%     30.074 MiB/s    +2.0348%           0.00  none
metrics_off/topology/tcp_socket/04_writers                                                           127.33 ms  -1.1411%     29.959 MiB/s    +1.1542%           0.01  none
metrics_off/topology/tcp_socket/08_writers                                                           256.41 ms  -4.5782%     29.755 MiB/s    +4.7979%           0.00  none
metrics_off/topology/tcp_socket/16_writers                                                           524.95 ms  -8.7029%     29.067 MiB/s    +9.5325%           0.00  none
...
metrics_on/topology/tcp_socket/01_writer                                                             51.630 ms  -1.6962%     18.471 MiB/s    +1.7255%           0.00  none
metrics_on/topology/tcp_socket/02_writers                                                            100.96 ms  -2.8973%     18.892 MiB/s    +2.9838%           0.10  none
metrics_on/topology/tcp_socket/04_writers                                                            207.15 ms  -2.5759%     18.415 MiB/s    +2.6440%           0.00  none
metrics_on/topology/tcp_socket/08_writers                                                            416.00 ms  -3.8642%     18.340 MiB/s    +4.0195%           0.00  none
metrics_on/topology/tcp_socket/16_writers                                                            841.09 ms  -6.9195%     18.142 MiB/s    +7.4339%           0.00  none

Looks like we even get some tangible performance improvement here! 🚀

@pablosichert pablosichert merged commit 45da52c into master Apr 2, 2021
@pablosichert pablosichert deleted the tokio-1.0 branch April 2, 2021 04:31
@binarylogic binarylogic changed the title chore: begin upgrade to tokio 1.x chore: Fully upgrade to tokio 1.x Apr 2, 2021
bruceg added a commit that referenced this pull request Dec 1, 2022
`tests/api.rs` has been entirely commented out since 45da52c (chore: begin
upgrade to tokio 1.x (#5872) April 2021), and `tests/enterprise.rs` will be
moving out as well.
jszwedko pushed a commit that referenced this pull request Dec 2, 2022
* chore(tests): Remove unused tests

`tests/api.rs` has been entirely commented out since 45da52c (chore: begin
upgrade to tokio 1.x (#5872) April 2021), and `tests/enterprise.rs` will be
moving out as well.

* Don't run enterprise tests
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.

None yet

8 participants