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::RenderService is 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

Edited by Stan Hu

Merge request reports

Loading
Loading