Introduce pagination on MergeRequests#discussions endpoint
Background
For large MRs with 100+ discussions/notes, it can still take more than a second to load when serialized response is cached and more if not cached. It's understandable because even when the JSON response is cached, the post processing and policy checks will still need to run on each of those 100+ discussions/notes. This was found out in #335566 (comment 667218572).
Proposal
Follow the same pagination approach we did for diffs by having a diffs_batch.json
but this time for discussions (something like discussions_batch.json
. This way, one big request will be split into multiple smaller requests though still sequentially loaded.
Pros
- We'll be able to show discussions by chunks resulting to faster response times.
- Faster response time means we'll be able to show discussions quicker bit by bit. This leads to better UX in my opinion since user can see discussions earlier than seeing a loading state.
- Faster response time means a Puma worker won't be blocked for a long time so it'll have time to perform other requests.
- Apdex score of this endpoint will improve since it won't take long anymore.
Cons
- Additional complexity because paginating this resource isn't that straightforward it seems.
- It's just splitting the big request to smaller request but they can't be loaded in parallel.
Related to #335566 (closed).