Skip to content

WIP: Stream the JSON array for the discussions endpoint

Nick Thomas requested to merge stream-controller-discussions-json into master

What does this MR do?

HAXXX

This MR cuts through several layers of abstraction to stream each element of a JSON array to the client as it is generated. We do this for the discussions endpoint, which can easily reach 1MiB or greater response body sizes. It's inspired by a discussion about streaming JSON with @robotmay, here: #217633 (comment 341937061)

The advantage of doing it like this is that we don't need to integrate a streaming JSON library like YAJL. Instead, we generate each element of the array separately, and push them out in chunks.

The disadvantage is... well. The code is not super-pleasant.

Right now, this doesn't save us very much time at all when generating the JSON response body, because we still build all the discussions before starting into a Ruby array of Ruby hashes, and that dwarfs the conversion time. We're also still using the original JSON encoder for each to_json call. Eventually, we could plug YAJL into it, and give YAJL itself an enumerator, to get relatively-clean code that is fully streamable.

To make this worthwhile, we'd need to do generation as well as conversion item-by-item.

Right now I'm testing and benchmarking against localhost, which minimises the benefits. On slow connections, the sooner we can start sending the bytes, the better.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Nick Thomas

Merge request reports