styx icon indicating copy to clipboard operation
styx copied to clipboard

Optimise bodiless request/response processing

Open mikkokar opened this issue 5 years ago • 0 comments

Description

A bodiless HTTP request consist of

  • HTTP headers, with Content-length: 0.
  • An empty ByteStream that emits an onComplete event only.

Styx relies on asynchronous onComplete event to detect that the request has been fully processed, and no more events are to be expected. It doesn't look at the Content-Length header.

However it could easily look into a Content-Length header to see that no more events are forthcoming without having to wait for ByteStream to complete. Having to wait for ByteStream to complete creates a window of opportunity for the remote end to respond before Styx has internally finished with the request. This is the scenario:

  1. Styx proxies request headers with Content-Length: 0 to the origin. It is still waiting for the ByteStream to complete.
  2. Remote end receives the message headers. It knows the message has been received in full because Content-Length: 0 header. The remote end responds.
  3. Styx receives the response, and proxies it back to the browser. But Styx still thinks the request is ongoing because the request ByteStream is yet to be completed.
  4. Styx thinks that "origin responded too soon" and closes the origin bound TCP connection instead of returning it back to the pool.

Impact

  • Suboptimal use of pooled resources. Styx ends up closing TCP connections to the origin, even if they can be returned back to the pool.

  • This situation tends to occur in load test environment. Less so in typical production setting.

AOB

Notice that the above text discusses about bodiless requests (Content-Length: 0 case). But this also happens with any content length greater than zero. The problem is the late arrival of onComplete event from ByteStream, which always creates a window of opportunity for an origin to respond before we have internally processed the request.

However this problem tends to be associated with Content-Length header only. With chunked transfer encoding, the origin needs to wait for the terminating chunk that is generated from the onComplete event. Therefore eliminating the window of opportunity.

A proper fix is for styx to count the request body bytes to detect the end of the request body, instead of relying on onComplete event alone.

mikkokar avatar Feb 05 '20 14:02 mikkokar