Render only first note when replying to a discussion
What does this MR do and why?
Replying to a discussion via POST .../merge_requests/:iid/discussions/:discussion_id/notes
loaded and Banzai-rendered every note in the discussion just to read the first
one. The cost grew linearly with the thread size, so a long thread made each
reply O(N) and the whole thread O(N²). This changes the handler to load only the
first note and check its readability, rendering nothing.
Root cause and details
In the POST route (lib/api/discussions.rb), only notes.first and
notes.empty? were used, but readable_discussion_notes loads all N notes
and prepare_and_filter_notes → prepare_notes_for_rendering runs four
preloaders and full Banzai Markdown rendering of every note, then an O(N)
readable_by? filter. All of it was discarded except element [0].
So reply #k rendered the k-1 earlier notes; across a thread that is O(N²). Diff notes make each render heavier.
It also explains the behavior at the per-noteable limit
(Noteable::MAX_NOTES_LIMIT = 5000): once an MR hit the cap, every further POST
still ran the full render before Note#does_not_exceed_notes_limit? rejected
it, so a bot retrying against a maxed thread paid the full render on each refused
request.
Observed on one MR: 5021 notes, one diff discussion holding ~4529 of them, almost all from a single bot.
Fix
The handler only needs the first note (for to_discussion,
can_reply_to_system_note?, part_of_discussion?, can_convert_to_discussion?)
and whether the user can read it. It now loads just that note and calls
readable_by?, matching the work items API (lib/api/work_items/discussion_notes.rb).
No notes are rendered on this path.
readable_by? needs no rendering for regular notes; for a system note it uses
all_references, which costs about one note and this endpoint rejects system-note
replies anyway. Dropping the old render-then-filter cannot change behavior:
confidentiality is uniform across a discussion and these noteable types reject
replies to system notes, so a thread whose root is unreadable but whose replies
are readable is not reachable.
The readable_discussion_notes helper is unchanged and still used by the GET
discussion endpoints, which legitimately render all notes for presentation.
Benchmarks
From the reproducer in https://gitlab.com/gitlab-org/gitlab/-/work_items/634294, CPU per reply POST (best of 3), each note referencing 5 issues and mentioning 1 user:
| Notes in thread | Total CPU before | Total CPU after |
|---|---|---|
| 1 | 503 ms | 478 ms |
| 100 | 649 ms | 478 ms |
| 400 | 1,404 ms | 537 ms |
| 1,000 | 3,034 ms | 632 ms |
Test
- Replying to a thread with many notes renders no notes (
Notes::RenderServiceis never instantiated). - An unreadable (confidential) discussion returns 404.
- The existing discussions API shared examples (reply, quick-action reply, system-note reply, individual-note conversion, permissions) continue to cover correctness.
Related to https://gitlab.com/gitlab-org/gitlab/-/work_items/634293 Related to https://gitlab.com/gitlab-org/gitlab/-/work_items/634294