-
Notifications
You must be signed in to change notification settings - Fork 2k
Description
Since at least 2020, Dapr has been using the fasthttp library for the internal HTTP API server. An Architectural Decision Record from June 2020 (that is still in "proposed" status) contains information on this decision, which was based on the fact that, at least on synthetic benchmarks, fasthttp is optimized for performance and raw speed, primarily thanks to avoiding almost any memory allocation.
However, with more experience building Dapr and with the desire to push it forward, we are now uncovering a series of issues that could make it advisable to convert the internal HTTP server to be based on the Go standard library (net/http). It's likely that our use cases are not what fasthttp is meant for, and as we invest in things like streaming supports, the issues will only be growing. This is advice that was provided by the maintainer (it appears to be a single person now) of fasthttp too.
Although switching from net/http to fasthttp will have a negative performance impact, arguably this should be negligible in the context of Dapr. Each one of our handlers performs a lot of operations (and are in large part I/O bound) and a lot of allocations and spanning of goroutines, which mask the effect of any micro-optimization performed by fasthttp.
Note that fasthttp contains both a server and client, and Dapr uses both, although the client part plays a very small role.
I am categorizing the issues in 3 groups.
Problems with Dapr currently
Lack of context support
With fasthttp, each request does not get a context that is tied to the lifetime of the request.
Although each handler is passed a ctx
which does implement the context.Context
interface (and we use it a lot in Dapr, example), the lifecycle of that context is tied to the server's context and not the request's context. This seems to be an explicit design decision (although not clearly documented).
This is an issue for Dapr because if the user (app) cancels a request, we do not have a way to be notified of that and abort in-progress operations.
According to the fasthttp maintainer:
If people want to use contexts they really shouldn't be using fasthttp. When you are using contexts it shows that you don't need the performance increase fasthttp gives. If you really needed that performance you wouldn't be using contexts to begin with.
No support for HTTP/2
fasthttp does not include support for HTTP/2. This means application cannot use HTTP/2 to connect to Dapr, possibly missing out on the performance benefits of things like multiplexing, etc.
Problems with handling streams
At a high level, fasthttp has incomplete support for streaming.
For example, the fasthttp client does not support making requests and sending/reading streams. In the PR adding support for streams, I had no choice but to use net/http for outbound requests (in the pkg/channel/http package, for Dapr to invoke the app).
According to the maintainer, streaming support has never been a priority because the kind of apps fasthttp is designed for do not need streaming, as they work with small blobs of data:
fasthttp is all about performance and it achieves this by not supporting these kinds of things. Streaming of response bodies doesn't make sense in the use cases fasthttp is for. If you need body streaming it's better to use net/http.
More specifically, even with the limited support for streaming, a lot of things are left to the developers (and are generally un-documented):
No explicit support for streaming response body from the server
In a fasthttp server, responding with a stream is not something that is available as built-in, and requires "hijacking" the request. Example:
Lines 109 to 138 in bcb202c
// withStream is like "with" but accepts a stream | |
// The stream is closed at the end if it implements the Close() method | |
func withStream(code int, r io.Reader, onDone func()) option { | |
return func(ctx *fasthttp.RequestCtx) { | |
if len(ctx.Response.Header.ContentType()) == 0 { | |
ctx.Response.Header.SetContentType(jsonContentTypeHeader) | |
} | |
ctx.Response.SetStatusCode(code) | |
// This is a bit hacky, but it seems to be the only way we can actually send data to the client in a streamed way | |
// (believe me, I've spent over a day on this and I'm not exaggerating) | |
ctx.HijackSetNoResponse(true) | |
ctx.Hijack(func(c net.Conn) { | |
// Write the headers | |
c.Write(ctx.Response.Header.Header()) | |
// Send the data as a stream | |
_, err := io.Copy(c, r) | |
if err != nil { | |
log.Warn("Error while copying response into connection: ", err) | |
} | |
if rc, ok := r.(io.Closer); ok { | |
_ = rc.Close() | |
} | |
// c is closed automatically | |
if onDone != nil { | |
onDone() | |
} | |
}) | |
} | |
} |
(Issue opened in fasthttp's repo)
Support for reading requests as a stream seems not mature
The only area where fasthttp does have "complete" support for streams seems to be in reading the request body in the server. However, that was a contribution from the community and it seems implemented in a spotty way. At a high level:
- Even when streaming is enabled, some requests do not come in as streams, so trying to read them with the methods to get a stream returns no data.
- When the request body is coming in as stream, the "max request body size" is ignored
Examples of "workarounds" required:
Lines 2478 to 2487 in bcb202c
// Reads the request body in full, supporting streams as well as in-memory bodies | |
func (a *api) readBody(reqCtx *fasthttp.RequestCtx) ([]byte, error) { | |
if s := reqCtx.RequestBodyStream(); s != nil { | |
if a.maxRequestBodySize > 0 { | |
s = io.LimitReader(s, a.maxRequestBodySize) | |
} | |
return io.ReadAll(s) | |
} | |
return reqCtx.PostBody(), nil | |
} |
Even more, support for using streams seems not fully stable. In the PR adding streaming support, E2E tests seems more flaky with occasional "EOF" errors. (It appears that it is not just us: 1 2 3)
General consideration
Lastly, a few general considerations:
- As the maintainers made it very clear (and are not shy of repeating to everyone in the repo), fasthttp is optimized for certain use cases that are not really aligned with what we are doing. It seems to shine in situations where it's dealing with small amounts of data and the response is generated synchronously (perhaps apps that closely resemble "helloworld-style benchmarks"). This is different from Dapr, where each handler can take a lot of time processing the request, makes a lot of allocations, and is generally I/O-bound
- The code of fasthttp is very complex and documentation is not always clear (see how we've been using the contexts "wrong" all this time). This is especially true if you are looking at the code that implements the (partial) support for streams.
- Lastly, from a risk perspective, net/http is maintained by the Go team and receives lots of attention. It is tested heavily in production in a variety of scenarios, and it is the target of extensive scrutiny also for security vulnerabilities. At this time, it appears that fasthttp has a single, part-time maintainer. While not uncommon in the OSS space, and not necessarily a deal-breaker per se, given the extent to which Dapr depends on this library, and the fact that servers are often a high-risk point for attacks, it's a point worth considering too in the risk calculations.