Skip to content

Improve performance of discussions.json action for Projects::MergeRequestsController under load into next tier

Related #30507 (closed)

Performance testing is showing that the Projects::MergeRequestsController#discussions.json controller and action is over our target of 500ms under load:

█ Results summary

* Environment:                10k
* Environment Version:        13.13.0-pre `48b385ebb2c`
* Option:                     60s_200rps
* Date:                       2021-05-26
* Run Time:                   3m 20.02s (Start: 10:28:38 UTC, End: 10:31:58 UTC)
* GPT Version:                v2.8.0

NAME                              | RPS  | RPS RESULT        | TTFB AVG  | TTFB P90            | REQ STATUS     | RESULT
----------------------------------|------|-------------------|-----------|---------------------|----------------|----------------
web_project_merge_request         | 20/s | 14.14/s (>6.40/s) | 1496.01ms | 5768.76ms (<7500ms) | 100.00% (>99%) | Passed

     http_req_waiting.................................................................: avg=1496.01ms min=130.24ms  med=258.31ms  max=10430.92ms p(90)=5768.76ms p(95)=6274.04ms
     ✓ { controller:Projects::MergeRequests::ContentController#cached_widget.json }...: avg=236.36ms  min=130.24ms  med=159.36ms  max=3918.39ms  p(90)=229.00ms  p(95)=745.00ms
     ✓ { controller:Projects::MergeRequests::ContentController#widget.json }..........: avg=235.67ms  min=138.49ms  med=172.64ms  max=3624.41ms  p(90)=275.59ms  p(95)=444.28ms
     ✓ { controller:Projects::MergeRequestsController#discussions.json }..............: avg=5935.96ms min=5201.95ms med=5762.25ms max=10430.92ms p(90)=6664.20ms p(95)=7059.64ms
     ✓ { controller:Projects::MergeRequestsController#show.json }.....................: avg=341.05ms  min=205.22ms  med=232.61ms  max=4082.58ms  p(90)=318.39ms  p(95)=478.23ms
     ✓ { controller:Projects::MergeRequestsController#show }..........................: avg=729.69ms  min=349.91ms  med=422.82ms  max=6461.13ms  p(90)=1284.01ms p(95)=1894.62ms

The results above show that the discussions.json action for the Projects::MergeRequestsController controller has a P90 of 6851.40ms. This was tested on our 10k Reference Architecture with a RPS target of 20/s. It targeted an MR on our own gitlabhq project that has a typically high number of comments - 119.

As per our new incoming performance targets this endpoint is above our main target of 500ms and falls under the ~S2 tier. Task is to improve the endpoint's performance into the next tier ~S3 (<2000ms).

Description of Frontend Effort

Feature flag: paginated_notes

Problem to solve

In order to improve the performance of discussions loading, backend has shipped !34628 (merged) that implements #217673 (closed)

frontend now needs to be updated to leverage this new way of loading discussions.

Please bear in mind the initial goal of improving the situation described here: #209784 (closed)

User experience goal

Faster load times.

Not affecting experience when it comes to native Find in page, etc. (ie, no Load more buttons).

Bear in mind the variations regarding the order of rendering, etc.

Beware also that this will cross over to the Issues.

Further details

See:

OK, the backend portion of this is completed now, gated behind the :paginated_notes feature flag. The frontend part of it, as prototyped in !33619 (closed) , can be worked on at any time, so I'll mark this as frontend now (cc @andr3).

Paginating the notes endpoint is useful all by itself, so we can work to remove the feature flag for that in the usual way.

@nick.thomas in #217673 (comment 380981019)

Permissions and Security

Documentation

Availability & Testing

What does success look like, and how can we measure that?

What is the type of buyer?

Is this a cross-stage feature?

Yes. Heads up to @donaldcook in devopsplan.

Links / references

Edited by Grant Young