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

Building server with several services hangs compiler in rust 1.56 #869

Closed
ricnewton opened this issue Dec 19, 2021 · 6 comments
Closed

Building server with several services hangs compiler in rust 1.56 #869

ricnewton opened this issue Dec 19, 2021 · 6 comments

Comments

@ricnewton
Copy link

Bug Report

Version

Tonic master (commit e78a2d8)

Platform

Reproduced on 64 bit Linux and 64 bit Windows.

Description

We have a tonic based api gateway intended to add a new grpc interface to a large amount of legacy code, we create a tonic server like:

Server::builder()
    .add_service(test_server::TestServer::new(Svc))
    .add_service(test2_server::Test2Server::new(Svc))
    ...
    .serve("127.0.0.1:1339".parse().unwrap());

We currently have on the order of 10-15 services we add, when going from rustc 1.55 to rustc 1.56 the compiler now seems to hang for many minutes.

I have added an integration test shown the issue here: https://github.com/ricnewton/tonic/blob/router_with_many_services/tests/integration_tests/tests/router.rs#L254

When doing

rustup default 1.55
cargo test --test router

The code compiles (though test router_with_35_services fails with a compiler error), when doing

rustup default 1.56
cargo test --test router

The compiler hangs for several minutes.

Is this the correct way to do this many services on tonic? Or is there a better way we should be using?

FYI I also tried the tests on branch port-tonic-router-to-axum and the tests pass there: https://github.com/ricnewton/tonic/blob/axum-router-test/tests/integration_tests/tests/router.rs#L254

@davidpdrsn
Copy link
Member

davidpdrsn commented Dec 19, 2021

This is a known regression in Rust 1.56. There is currently no workaround other than downgrading to 1.55. The compiler team is aware of it and working on it.

@ricnewton
Copy link
Author

ricnewton commented Dec 19, 2021

So looking into how the router works with the nested generics stuff, do you expect the current approach to scale to more than a few 10s of services though?

Scaling to just 15 services seemed to fail even with 1.54 (doesn't hang but you get a compiler error of error[E0275]: overflow evaluating the requirement).

Can we expect to be able to have many 10s of services supported?

@davidpdrsn
Copy link
Member

Yes any number of services should work and if it doesn't its a bug. Whether the bug is in rust or tonic however, is sometimes hard to say.

I also maintain axum where we've gone to great lengths to make sure things compile quickly but it requires making different trade-offs especially around what you choose to box.

We're considering moving tonics router to axum (#830) internally which would fix these issues. However thats a breaking change so we don't want to rush it. The milestone for the next breaking release is here.

@ricnewton
Copy link
Author

Yeah, as I said above in the report, the axum branch seemed to work fine, I will try to test our actual api gateway with that.

The current branch though seems to create a new nested type per service you add, so you end up with Routes<Test14, ...Routes<Test13,.. <Routes<Test12,...>>>, that seems to be really limited. It also mean the supported services are statically known at compile time, there is no way to dynamically add a service by the looks of it, that could be useful.

@davidpdrsn
Copy link
Member

Its just a matter of trade-offs. Being able to add services dynamically (which is possible by writing your own tower::Service) was never a goal so the API wasn't designed with that goal in mind.

Regardless, I think I'll close this issue now since there is nothing we can do about it outside what we're already doing.

@ricnewton
Copy link
Author

OK, sounds good, I'll see if the axum branch works for us. Thanks.

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

No branches or pull requests

2 participants