Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Upgrade to axum 0.7 #7670

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Upgrade to axum 0.7 #7670

merged 3 commits into from
Dec 7, 2023

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Dec 6, 2023

This PR upgrades our projects to axum 0.7 and hyper 1.x.

The changes on the axum side are described by https://tokio.rs/blog/2023-11-27-announcing-axum-0-7-0

Unfortunately there appears to be no way to adopt most of these breaking changes incrementally, so this PR has gotten rather large. The three main changes are:

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Dec 6, 2023
@Turbo87 Turbo87 requested a review from a team December 6, 2023 14:59
@syphar
Copy link
Member

syphar commented Dec 6, 2023

Just a note, I found that the graceful shutdown impl felt a little complex in the example, and wanted to see if tokio-rs/axum#2398 lands quickly enough

@Turbo87
Copy link
Member Author

Turbo87 commented Dec 6, 2023

@syphar thanks for that link. it would certainly make this a lot easier... I'm not sure whether that supports ConnectInfo though?

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

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

Having just gone through this with another project, this LGTM, but I'd be so much happier if we didn't have to do the graceful shutdown dance in our own, non-library code. (Obviously nothing we can do about that right now.)

I think the key here is going to be testing this pretty thoroughly on staging — areas in particular I think we'll want to poke at are behaviour under load and that service restarts still work reliably. (I know this is basically all taken verbatim from the example, and it all reads fine to me, but they're the areas I'm a little nervous about, personally.)

src/bin/server.rs Show resolved Hide resolved
@Turbo87
Copy link
Member Author

Turbo87 commented Dec 7, 2023

behaviour under load

I've tested it on staging using ApacheBench and the numbers didn't look significantly different before/after deploying this branch. The production dyno setup is a bit different to staging, but it looks like performance should be roughly similar to before.

service restarts still work reliably

I've used heroku restart -a staging-crates-io and according to the logs the graceful shutdown worked as intended and the download counters were flushed before exiting the process.

@Turbo87 Turbo87 merged commit 9e17c20 into rust-lang:main Dec 7, 2023
6 checks passed
@Turbo87 Turbo87 deleted the axum-0.7 branch December 7, 2023 12:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants