panic: Drain() is not implemented

[heyitsanthony]

Please answer these questions before submitting your issue.

What version of gRPC are you using?

v1.4.2

What version of Go are you using (go version)?

v1.8.3

What operating system (Linux, Windows, …) and version?

Linux 4.11.9

What did you do?

etcd shuts down grpc by issuing a graceful stop, then a stop() if streams are holding it open via:

ch := make(chan struct{})
go func() {
   defer close(ch)
   gs.GracefulStop()
}()
select {
case <-ch:
case <-time.After(timeout):
   // close dangling streams
   gs.Stop()
    <-ch
}

What did you expect to see?

No panic.

What did you see instead?

../bin/etcd_test-31456: panic: Drain() is not implemented
../bin/etcd_test-31456:
../bin/etcd_test-31456: goroutine 233 [running]:
../bin/etcd_test-31456: google.golang.org/grpc/transport.(*serverHandlerTransport).Drain(0xc4201a6720)
../bin/etcd_test-31456:         /home/anthony/go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/transport/handler_server.go:384 +0x64
../bin/etcd_test-31456: google.golang.org/grpc.(*Server).GracefulStop(0xc420128000)
../bin/etcd_test-31456:         /home/anthony/go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/server.go:1066 +0x2e6
../bin/etcd_test-31456: github.com/coreos/etcd/embed.(*Etcd).Close.func2(0xc4201d9680, 0xc4201ba128)
../bin/etcd_test-31456:         github.com/coreos/etcd/embed/_obj/etcd.go:221 +0x5e
../bin/etcd_test-31456: created by github.com/coreos/etcd/embed.(*Etcd).Close
../bin/etcd_test-31456:         github.com/coreos/etcd/embed/_obj/etcd.go:222 +0x26a

/cc @gyuho

[dfawley]

There are a number of features that are not supported or available when serving via ServeHTTP(), and this is one of them.

The implementation for Drain() in grpc's transport sends a GOAWAY to the client. In a multitenancy environment (ServeHTTP()), this would be inappropriate (if it is even possible), as it would affect the other handlers. The gRPC spec has no other way to notify the client that it should stop sending new RPCs.

The right way to do what you want is to perform a graceful stop on the http server (using Shutdown) instead of the gRPC server. Alternatively, you can use the standard Serve(net.Listener) function instead of ServeHTTP(), which has many advantages (e.g. performance).

We may look into better support for the HTTP server handler model, possibly by hijacking the net.Conn away from the http server, but it's not straightforward.

In terms of what to do with ServeHTTP()+GracefulStop() (because I don't particularly like the panic, either), I'm not sure there is much we can do. If we don't panic, there is no other way to report an error back to the caller. And we don't return an error, so if we silently can't perform the operation (tell the client to stop sending new RPCs), then that would be bad, too. So it seems panic is the only option.