Should raw low-level request handlers provide a Writer?

[abhinav]

We could have the low-level handler provide a ResponseWriter (similar to http's ResponseWriter) which will let us skip the io.Copy we have to do from the response Reader to the http.ResponseWriter.

[HelloGrayson]

Curious what @prashantv and @akshayjshah have to say here.

[prashantv]

I'm a little biased since I suggested this in a code review.

The HTTP ResponseWriter buffers already, as does TChannel (it writes into a frame, and then sends that frame). Since the underlying layers already buffers, if we introduce another buffer in our transport layer, we're doing 2x the buffering, and (possibly more importantly) twice the memory allocations.

While I'm not too concerned about performance currently, this kind of change is would break the design / API so I want to make sure we think about this now rather than later.

[abhinav]

True. Another reason I'm seriously considering it is this: For Thrift (and other encoders) will all have some sort of WriteTo(io.Writer) function, needing to return an io.Reader for the body requires another buffer, but if we have something that has a Writer, we can forego that allocation there as well.

[akshayjshah]

I'm all in favor of this. Even if we're not worried about performance, using an http.ResponseWriter-type interface is familiar in Go. The more familiar we can make YARPC feel, the less foot-shooty it'll be.

[abhinav]

FWIW this won't affect the public API for JSON and Thrift but it will affect the public API for raw-low level usage (streams of raw bytes, forwarding servers, or implementing your own encoding).

[HelloGrayson]

🍔 🍔 foot-shooty 🍔 🍔