Repository navigation
RequestBodyBufferMiddleware should limit total memory consumption for concurrent requests #255
Description
The RequestBodyParserMiddleware currently accepts a limit on the maximum size for a single request (say 100MIB). If you spawn lots and lots of concurrent requests, you can easily take up all available memory. As such, this middleware should probably also take care of limiting total memory consumption for all concurrent requests.
Activity
WyriHaximus commentedon Nov 22, 2017
Fixing #218 would ideally solve this issue and I rather solve that there then in
RequestBodyBufferMiddlewareas I'm a) not sure how to solve that inRequestBodyBufferMiddlewareand b) I'm afraid we're going to end up with incomplete bodies when we start ignoring chunks.clue commentedon Nov 23, 2017
I'm currently looking into updating #218 and I agree that this can be used to avoid this issue. However, this will only limit the number of concurrent requests and thus only sets an upper bound limit on the total memory consumption and can not actually take care of actual total memory consumption.
For example, say we use the upcoming
RequestLimitHandlerset to 100 concurrent requests with each max 10 MB we can take up to 1000 MB max concurrently. On the other hand, if we control this 1000 MB max concurrently in theRequestBodyBufferMiddleware, we can actually process many more requests concurrently as long as they are smaller. Under the reasonable assumption that most requests do not fill this maximum, this allows us to achieve a much better concurrency.WyriHaximus commentedon Nov 23, 2017
Fair point, but if #218 can be done, we can implement that tech into
RequestBodyBufferMiddlewareand handle it there as wellclue commentedon Nov 23, 2017
Not sure if that's reasonable, but you're raising a valid point! Arguably, we're discussing a premature optimization here, so I would suggest getting #218 in first and then look into this as a separate feature. Removing the milestone for now 👍