Highlight suggestions without a Gitaly compare per discussions load

SuggestionEntity resolved each suggestion's diff file through Suggestion#diff_file, which runs a Gitaly compare (two FindCommit plus one CommitDiff) for every suggestion-bearing note on every discussions.json load. On 2026-09-16 that call was what Gitaly's per-repository CommitDiff limiter rejected on gitlab-org/gitlab, so MR discussions failed to load (https://gitlab.com/gitlab-com/gl-infra/production/-/issues/22925).

Highlighting only needs paths, diff refs and blob sizes, so the entity now uses the discussion's stored note_diff_files row via a new diff_file_for_highlight, which also covers suggestions in replies. Applying suggestions still uses the latest diff file. Behind suggestion_highlight_uses_note_diff_file (disabled by default, rollout #629414).

Only syntax highlighting of the suggestion preview can differ, in two edge cases:

Case Flag off Flag on
File grew past the highlight size limit after the comment, position tracked to head (repro with limit lowered to 1 KB) off_grow on_grow
File renamed after the comment (position tracking fails on renames, so both states resolve the same original file) off_rename on_rename
Detailed context for AI agents

Incident background

On 2026-09-16, GitLab.com MR discussions on gitlab-org/gitlab failed to load with GRPC::ResourceExhausted ("maximum queue size reached") from Gitaly's per-repository CommitDiff concurrency limiter on gitaly-cny-01. Rails renders this as a 503 via the ApplicationController rescue. Production backtraces all ended in suggestion_entity.rb:9 -> Suggestion#diff_file -> DiffNote#latest_diff_file -> Gitlab::Diff::Position#diff_file -> CommitDiff. Related incident: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/22925. Much of the traffic filling the queue was unauthenticated crawler traffic loading discussions.json on old MRs, but the same call runs for every human viewer on Projects::MergeRequestsController#discussions, deduplicated only per file within a single request.

Root cause

SuggestionEntity (app/serializers/suggestion_entity.rb) calls Gitlab::Diff::Highlight with a diff file obtained from Suggestion#diff_file. That resolves via DiffNote#latest_diff_file -> Gitlab::Diff::Position#diff_file(repository) -> CompareService -> Gitlab::Git::Compare, costing two Gitaly FindCommit RPCs plus one CommitDiff RPC per suggestion-bearing note, on every load.

Fix

New method diff_file_for_highlight:

  • Default implementation lives in the Suggestible concern (app/services/concerns/suggestible.rb) and returns diff_file, so Gitlab::Diff::Suggestion (used by markdown preview and draft publishing) is unchanged.
  • Suggestion (the persisted model) overrides it to return note.discussion.diff_file when the flag is on, backed by the stored note_diff_files row in Postgres, needing no Gitaly compare. note_diff_files rows are only ever created for the first note of a discussion (DiffNote#should_create_diff_file? requires start_of_discussion?), so a reply's own note never has one; replies copy the first note's original_position (via DiffDiscussion#reply_attributes), so the first note's stored row is valid for them too, and it is the same memoized Gitlab::Diff::File DiscussionEntity already built for the discussion, no extra query or Gitaly call. With the flag off the method still returns diff_file (note.latest_diff_file) before touching the discussion, so flag-off behaviour is unchanged.
  • SuggestionEntity now calls diff_file_for_highlight instead of diff_file.
  • Suggestion#diff_file itself is unchanged, so applying suggestions, outdated?(cached: false), Gitlab::Suggestions::SuggestionSet, and FileSuggestion keep using the latest diff file.

What Highlight actually needs from the file

Gitlab::Diff::Highlight only needs: diff_refs (truthiness), old_path/new_path (lexer selection), old/new blob sizes (the "too large to highlight" guard, Gitlab::Highlight.file_size_limit, default 512 KB, from maximum_text_highlight_size_kilobytes), and the repository (for the gitlab-language gitattributes override). The stored note diff file provides all of these. The blob lookups use the same BatchLoader keys as the discussion's own diff file, so no additional Gitaly RPC is added by this change.

Feature flag

suggestion_highlight_uses_note_diff_file, type gitlab_com_derisk, default disabled, actor is the note's project. Rollout issue: #629414. When disabled, behaviour is exactly as before.

Behaviour differences with the flag on (cosmetic, highlighting only)

  1. Lexer follows the file path at comment time. Only matters if a later rename changed the file extension. In practice this cannot diverge today: renames after a comment make position tracking fail (a pathspec-limited diff does not see the renamed target), so the note stays outdated at its original refs and latest_diff_file already resolved to the same original file. Verified on GDK: identical rendering with the flag on and off (screenshots off_rename / on_rename).
  2. The too-large guard checks blob size at comment time instead of current head. If a file crossed the highlight size limit after the comment and the position was tracked to head, flag-off rendering was plain text and flag-on rendering is syntax highlighted (or vice versa if the file shrank below the limit). Verified on GDK by lowering the limit to 1 KB and growing a file from about 150 bytes to about 3 KB after commenting: flag off renders data-lang="plaintext", flag on renders data-lang="ruby" (screenshots off_grow / on_grow).
  3. Discussions with no note_diff_files row on the first note (worker not yet run, or failed) still fall back to the same CommitDiff path, via DiffDiscussion#diff_file -> first_note.diff_file. Replies are no longer a separate gap; only this case remains. Not addressed here.
  4. One new, smaller cost with the flag on. The notes polling endpoint (NotesActions#index, using the note serializer) renders SuggestionEntity without a Discussion wrapper, so note.discussion there costs two indexed Postgres queries per suggestion-bearing note (one for the discussion's notes, one for the stored row) instead of a Gitaly compare. Polling only returns notes created since the last poll, so this is small and infrequent.

Rollout observation

discussions.json responses carry an ETag derived from discussion cache keys, which does not include the flag state. A browser holding a cached copy gets a 304 and keeps the old rendering until a note on the MR changes. This is harmless but explains why flipping the flag does not visibly change an already-open MR.

Alternatives rejected

  • Build Highlight from a path and language only, no file object. Changes Gitlab::Diff::Highlight's contract and drops the gitattributes override.
  • Make latest_diff_file read Postgres when the position matches the MR's current diff refs. Would help all callers, but only for non-outdated notes, and touches write paths. Better as a follow-up.
  • Rescue ResourceExhausted in the entity and render plain text. Still burns the Gitaly call before failing.
  • Cache highlighted suggestion lines in Redis. More machinery than simply removing the call.

Follow-ups (not in this MR)

  • Note creation also hits CommitDiff synchronously via Discussions::CaptureDiffNotePositionService in Notes::CreateService#when_saved, and fails after the note is already saved.
  • note_diff_files backfill for old notes.
  • Evaluate the stale? ordering in IssuableActions#discussions and an ETag caching router route.

Verification

  • bundle exec rspec spec/serializers/suggestion_entity_spec.rb spec/models/suggestion_spec.rb -> 38 examples, 0 failures. The model spec stubs the discussion diff file with an instance_double and asserts the returned object, and adds a real-record case with a first note plus a reply asserting CompareService is never built and the reply gets the first note's stored row; the entity spec asserts that whatever diff_file_for_highlight returns is what gets passed to Gitlab::Diff::Highlight, with the flag cases living in the model spec.

  • RuboCop clean on the three changed files.

  • Manual GDK verification as described above, on gitlab-org/gitlab-test MR 3319, with three suggestion notes covering: a renamed file, a file grown past the diff patch limit so its position went untracked, and a file grown past the highlight size limit with its position still tracked.

  • Measured on GDK with a first-note suggestion and a reply-note suggestion on the same file, after a push to the source branch so the notes were no longer at the merge request's current diff refs. Gitaly RPCs per discussions.json render:

    Scenario Gitaly RPCs
    Flag off 5 (2 FindCommit, 1 CommitDiff, 1 GetBlobs, 1 GetInfoAttributes)
    Flag on, first revision of this MR 5 (the reply still compared)
    Flag on, this revision 0

    The reply's highlight file is the first note's stored row (same unique_identifier), and highlighting still produces rich lines.

Rollout

Flag suggestion_highlight_uses_note_diff_file is disabled by default. Rollout issue: #629414. Enable for gitlab-org/gitlab first before wider rollout.

Edited by Marc Shaw

Merge request reports

Loading
Loading