Skip to content

Conversation

@rnewson
Copy link
Member

@rnewson rnewson commented Sep 4, 2020

Overview

When set, every response is sent once fully generated on the server
side. This increases memory usage on the nodes but simplifies error
handling for the client as it eliminates the possibility that the
response will be deliberately terminated midway through due to a
timeout.

The config value can be changed at runtime without impacting any
in-flight responses.

Testing recommendations

Set [chttpd] delay_until_end to true then confirm that streamed responses are now sent non-stream (i.e, with a Content-Length header and not in chunked transfer mode).

Related Issues or Pull Requests

N/A

Checklist

@rnewson rnewson force-pushed the delay_response_until_end branch from 667cf84 to ffa5946 Compare September 4, 2020 12:16
@janl
Copy link
Member

janl commented Sep 4, 2020

Neat! Have you considered making this a per-request option instead of a global config?

@rnewson
Copy link
Member Author

rnewson commented Sep 4, 2020

I'm open to that, sure, would not be hard to make it a request parameter as we have the original Req at the start.

@rnewson rnewson force-pushed the delay_response_until_end branch from ffa5946 to 4f03de1 Compare September 4, 2020 13:58
@rnewson
Copy link
Member Author

rnewson commented Sep 4, 2020

@janl done. I am interested in feedback on the name of this config setting and request param (i.e, I don't much like my choice but can't think of a better name).

@janl
Copy link
Member

janl commented Sep 4, 2020

OTOH:

  • buffer_response(s)
  • disable_streaming_response(s)

@rnewson
Copy link
Member Author

rnewson commented Sep 4, 2020

thinking I might reference "transfer encoding" explicitly, with "identity" and "chunked" instead of boolean?

@rnewson
Copy link
Member Author

rnewson commented Sep 4, 2020

enh, but the point of the change is the delay. not sending the response until we're sure we can send the whole thing, it's not really about the transfer encoding, that's a side-effect (it just seemed silly send a chunked response when I have the whole thing in memory and can calculate the content-length)

@rnewson rnewson force-pushed the delay_response_until_end branch from 4f03de1 to fb339a5 Compare September 4, 2020 16:40
@rnewson
Copy link
Member Author

rnewson commented Sep 4, 2020

buffer_response it is.

When set, every response is sent once fully generated on the server
side. This increases memory usage on the nodes but simplifies error
handling for the client as it eliminates the possibility that the
response will be deliberately terminated midway through due to a
timeout.

The config value can be changed at runtime without impacting any
in-flight responses.
@rnewson rnewson force-pushed the delay_response_until_end branch from fb339a5 to 881f52f Compare September 4, 2020 17:20
@rnewson rnewson merged commit c625517 into master Sep 7, 2020
@rnewson rnewson deleted the delay_response_until_end branch September 7, 2020 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants