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

FR: Port sharing with traditional HTTP services #3458

Closed
Capstan opened this issue Sep 13, 2017 · 17 comments
Closed

FR: Port sharing with traditional HTTP services #3458

Capstan opened this issue Sep 13, 2017 · 17 comments
Milestone

Comments

@Capstan
Copy link
Member

Capstan commented Sep 13, 2017

As of v1.6.1, there appears to be no way, when using gRPC with Netty, to host traditional HTTP services side-by-side with gRPC on the same port. I am working with an existing framework that wants to support gRPC but only has one port available and expects to also serve HTTP.

At the moment, the workaround would involve rearchitecting several disparate systems to know about split fleets of services or multiple ports, followed by some careful rollouts to migrate traffic. I'd like to avoid this if at all possible.

@ejona86
Copy link
Member

ejona86 commented Sep 14, 2017

There is no port sharing solution currently available. The only workaround is to expose an in-process reverse proxy that forwards content to the appropriate server. But that can have performance implications.

The long-term solution has been netty/netty#3667 . It's only just recently reached feature-parity to the point we could potentially migrate to it.

@pmasrani
Copy link

@ejona86: Is the work on this bug in progress or do we have a timeline on this fix?

@ejona86
Copy link
Member

ejona86 commented Apr 17, 2018

The Netty stuff has been progressing. There was a PoC to use the new Netty API, but we've not swapped to it and it will have bitrot to a degree.

@thekalinga
Copy link

@ejona86 Any luck with PoC?

@trustin
Copy link
Contributor

trustin commented Aug 1, 2018

Shameless plug: You can mix and match gRPC, Thrift, REST, Tomcat, Jetty and whatnot with Armeria. HTTP/1.1 fully supported. https://line.github.io/armeria/

@fkorotkov
Copy link

I think most of the cases will be covered if Java GRPC will be able to serve regular http 200 OK requests on / or /health if GRPC service implements grpc.health.v1.Health. Unfortunately, a lot of load balancers including the GCP one require such endpoints for health checking and they can't use another port. 😪

@ttj4
Copy link

ttj4 commented Dec 10, 2019

@ejona86 any luck with poc?

@ejona86
Copy link
Member

ejona86 commented Dec 10, 2019

@ttj4, one part of the netty piece is #2332. I made progress on it last year, but it still needed more work to fix the bitrot.

@fkorotkov
Copy link

fkorotkov commented Jul 29, 2020

Just a note for future researchers. I tried Armeria from @trustin's comment before and it didn't work quite well. I've tried it once again and nowadays it worked pretty well. Posting a few caveats that I've found while migrating to Armeria under a GKE Ingress (GCP HTTP Balancer):

  1. Had to set maxRequestLength(0) so streaming of large binary files in 2Mb chunks works
  2. Had to set requestTimeoutMillis(0) for long streams to not be canceled

Other than that all works great so far, can see better CPU utilization seems because of our use of Kotlin Coroutines and their own executors. grpc-java has a separate thread pool that we actually didn't use much because of Kotlin Coroutines and Armeria doesn't create it by default.

My full Kotlin code snippet
val serviceImpl = ...
val healthService = ...  // impl of HealthGrpc.HealthImplBase
val grpcService = GrpcService.builder()
      .addService(
        ServerInterceptors.intercept(
          serviceImpl,
          ExceptionLoggingInterceptor(),
          MonitoringServerInterceptor(rootStats.scoped("grpc"))
        )
      )
      .addService(healthService)
      // See https://github.com/grpc/grpc-java/blob/master/documentation/server-reflection-tutorial.md
      .addService(ProtoReflectionService.newInstance())
      .supportedSerializationFormats(GrpcSerializationFormats.values())
      .enableUnframedRequests(true)
      .build()

val grpcServer = Server.builder()
      .http(port)
      .https(port)
      // self signed certificate to make GCP HTTP/2 Load Balancer happy
      // see https://cloud.google.com/load-balancing/docs/ssl-certificates#backend-encryption
      // Generated via https://devcenter.heroku.com/articles/ssl-certificate-self
      .tls(File(certificatesFolder, "server.crt"), File(certificatesFolder, "server.key"));
      .maxRequestLength(0) // unlimited
      .requestTimeoutMillis(0) // unlimited
      .service(grpcService)
      .decorator(LoggingService.newDecorator())
      .serviceUnder("/") { _, _ -> HttpResponse.of(HttpStatus.OK, MediaType.ANY_TEXT_TYPE, "Please use gRPC ;-)") }
      .service(Route.builder().exact("/healthz").build()) { _, _ ->
        val errorMessage = healthService.refreshStatus()?.second?.message
        if (errorMessage != null) {
          HttpResponse.of(HttpStatus.INTERNAL_SERVER_ERROR, MediaType.ANY_TEXT_TYPE, errorMessage)
        } else {
          HttpResponse.of(HttpStatus.OK)
        }
      }
      .build()
grpcServer.start().join()

@ulfjack
Copy link
Contributor

ulfjack commented Dec 23, 2020

We think that it's unacceptable for our users to have to remember different ports for our gRPC and HTTP APIs (and we also don't want to use a different library), and we have created a couple of patches that allow us to respond to HTTP requests on the same port used by gRPC. Currently, our patches only work with netty. The question is, would you be willing to merge some or all of them?

@ejona86
Copy link
Member

ejona86 commented Dec 23, 2020

@ulfjack, we can look at the patches, but it is highly likely they require a large or brittle API which makes them unsuitable for merging. I suggest either giving us an overview or you send us a draft PR (so it doesn't need to be clean) which would allow a low-cost pre-review.

@piotr-sumo
Copy link

@ejona86 could you elaborate more on the in-process reverse proxy? We'd like to experiment with that and run some tests in order to check wether or not we can pay the performance price.

ulfjack added a commit to EngFlow/grpc-java that referenced this issue Mar 14, 2021
This is a fairly hacky way of allowing gRPC and (normal) HTTP2 calls to
coexist on the same port. If an incoming HTTP2 call is *not* a gRPC
call, we simply forward it to an application-provided handler. This is
exclusive to netty. It also adds some API surface to the gRPC Java
library that may be undesirable from a maintenance point of view.

There are a few hacks here:
- Injection of the handler through a static field (this could be moved
  to the netty server builder)
- Extension of NettyServerStream.TransportState, and calls into private
  methods of the super class
- Reuse of the SendGrpcFrameCommand class with non-gRPC frames (I would
  argue that the class should be renamed, as its not at all specific to
  gRPC)

Another question is how flow control should be handled.

Related to grpc#3458.
ulfjack added a commit to EngFlow/grpc-java that referenced this issue Mar 14, 2021
This is a fairly hacky way of allowing gRPC and (normal) HTTP2 calls to
coexist on the same port. If an incoming HTTP2 call is *not* a gRPC
call, we simply forward it to an application-provided handler. This is
exclusive to netty. It also adds some API surface to the gRPC Java
library that may be undesirable from a maintenance point of view.

There are a few hacks here:
- Injection of the handler through a static field (this could be moved
  to the netty server builder)
- Extension of NettyServerStream.TransportState, and calls into private
  methods of the super class
- Reuse of the SendGrpcFrameCommand class with non-gRPC frames (I would
  argue that the class should be renamed, as its not at all specific to
  gRPC)

Another question is how flow control should be handled.

Related to grpc#3458.
@ulfjack
Copy link
Contributor

ulfjack commented Mar 14, 2021

I have a draft here: https://github.com/EngFlow/grpc-java

Any feedback is welcome.

@mdzhigarov
Copy link

Any update on this? Is port sharing still no supported?

@darren-fu
Copy link

I have a draft here: https://github.com/EngFlow/grpc-java

Any feedback is welcome.

what about this patch code, is it a nice way ?

@panchenko
Copy link
Contributor

It can work in Tomcat via the grpc-servlet(-jakarta) module.

@ejona86
Copy link
Member

ejona86 commented Feb 15, 2024

I'm going to close this as fixed by grpc-servlet and grpc-servlet-jakarta. I wouldn't be surprised if someone wants Netty port sharing specifically. I'd suggest opening a separate issue to track that.

Fixed by #8596

@ejona86 ejona86 closed this as completed Feb 15, 2024
@ejona86 ejona86 removed this from the Unscheduled milestone Feb 15, 2024
@ejona86 ejona86 added this to the 1.53 milestone Feb 15, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests