Limit the number of commits in push merge request emails
What does this MR do and why?
For #351293 (closed)
Recent, we noticed some Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError
exceptions after Sidekiq size limiter was enabled. There are multiple code paths leading to that exception. One of them is Notification#push_to_merge_request
. This method pushes an email notification with corresponding changed commits to the user. At the moment, we don't have any limitation for the commits. If the change contains a huge number of commits, for example when a merge request of gitlab-org/gitlab is rebased, the email content may grow very fast. That exceeds the size limit of Sidekiq job payload.
Therefore two optimization opportunities here:
- We can truncate the number of displayed commits in the email. The whole purpose of email notification is for human to read and react. I highly doubt that there would be anyone who relies on the full list of commits in the emails. In fact, to make the email exceed the limit, the commit list must be super long, maybe thousands of commits. That number is overwhelming for anyone to make sense from. This MR limits the latest 20 commits.
- We don't really use the full list of
@existing_commits
in the email content. The content requires the first, the last, and the number of existing commits to generate the following message:* ffa2983e...deafbf11 - 214 commits from branch master
. So, we don't properly need the full populated list. We can simply replace that list by three equivalent parameters.
After applying those two points, the size of email payload is always kept small. As a result, the aforementioned exception would never raise again.
Screenshots or screen recordings
Case 1: The number of new commits is smaller than 20
Case 2: The number of new commits is more than 20
Expected result: The email show the latest 20 commits and "And ... more" at the end. It also shows compare link of first and last existing commits
Case 3: When where is only one existing commit and some new commits
Expected result: The email show the latest 20 commits and "And ... more" at the end. It also shows the only existing commit and corresponding link.
Case 4: When where is only one existing commit and no new commit
Case 5: When there is no new commit and no existing commits
How to set up and validate locally
This is the basic functionality of Gitlab. To test on local environment, we can open a merge request. Then, push (force) to the branch correspondingly to the testing scenarios. The email notifications are captured at http://127.0.0.1:3000/rails/letter_opener/
.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #351293 (closed)