Skip to content

Make Commit#has_been_reverted? cheaper

Nick Thomas requested to merge 199197-make-has-been-reverted-check-cheaper into master

What does this MR do?

There is no way in git to efficiently tell if a commit has been reverted. Instead, GitLab creates system notes when we perform this action in the web UI. These are introspected to answer the question; doing so requires us to extract commit references from those notes.

With this change, we stop extracting commit references from the message of the commit that is under question. This is user-controlled content and we can't control the number of references in it. Further, since the revert happens after that commit message is written, there is no way for it to contain references to a reverting commit.

When the RevertService runs, it adds a note to the commit being reverted like "mentioned in commit abcdef", where abcdef is the ID of the reverting commit. That commit contains a message like "reverts commit "..."`. This is the data necessary for this method to return the correct value, and it's not affected in any way by this change.

Without this change, we get Gitaly N+1 and database N+1 issues when viewing a pathological commit, or an MR where the merge commit is pathological.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Closes #199197 (closed)

Closes #21049 (closed)

Edited by Nick Thomas

Merge request reports