Debounce GET requests for notes after submitting MR review
What does this MR do and why?
- In this discussion thread, the analysis identified that the GitLab frontend immediately starts to contineously pull notes in fast succession and partially in parallel after clicking the button "Submit review"
After clicking the button "Submit review", the GitLab frontend immediately starts to contineously pull notes in fast succession and partially in parallel, i.e. every 1-2 seconds (GET
http://gdk.test:3000/root/repro-many-diff-notes-in-mr-cause-hich-load/noteable/merge_request/133/notes
) => this leads to a large number of additional requests that are sent to the backend server => unfortunately, the backend is quite slow in responding and takes longer to respond than the requests are coming in. This leads to an ever increasing number of requests (/notes
) that are blocked => this causes high load on the server
- Why? The MR pages manages a vuex store that is responsible for fetching MR notes (real MR notes and not pedning draft notes); the vuex store initializes an ActionCable (websocket) subscription for the channel (
Noteable::NotesChannel
)- Why? In order to receive notifications from the backend when new MR notes have been added => this means, for each draft note converted into a real note the backend sends an update event through the ActionCable channel and triggers a new GET request in the frontend
- Why? The trigger comes from a hook
after_commit
in the modelNote
=> So the moreNote
entities are changed the moreupdate
events are sent through the websocket.
This MR intends alleviate the performance issue by debouncing the GET request for notes. We hope that this reduces the load on the backend server after a review with a lot of notes has been submitted.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
MR Checklist (@gerardo-navarro)
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the javascript style guides -
Conforms to the database guides
Screenshots or screen recordings
No change in frontend changes . This MR ensures that the frontend does not overstrains the backend with to many GET requests /notes
in a short amount of time (when a lot of MR draft notes are submitted).
Before | After |
---|---|
On the branch master , we see that the GET request is executed more than 20 times. The reason for the large number of GET request is documented in the MR decription. |
With this MR, we only reduce the number of GET requests (/notes ) considerably. In case of the video, there is only one GET request being set to the server. |
Loom video: https://www.loom.com/share/68d6f0c10a9c40819cee25e16e72ddf0 | Loom video: https://www.loom.com/share/b1c8cb933df24634bb7ffb842f8d06fb |
How to set up and validate locally
- Go to an open MR, e.g. http://gdk.test:3000/flightjs/Flight/-/merge_requests/4
- In the
rails console
, create 50 (or more) simple draft notes for this open MR
(1..51).each { |i| DraftNote.create(merge_request: merge_request, note: "Comment #{i}", author: User.find(1)) }
- Reload the page of the open MR
- Open the developer console and the tab "Network" to see the network requests
- Click the button "Finish review" and then the button "Submit review"
- After a few seconds, you will see that only a few GET requests (+/- 3 request depending on the current setup) are sent to the backend, see https://www.loom.com/share/b1c8cb933df24634bb7ffb842f8d06fb
- You can follow the same steps on the branch
master
and you will notice that more GET requests are sent to the backend, see https://www.loom.com/share/68d6f0c10a9c40819cee25e16e72ddf0
Related to #433461 (closed)