Migration Analysis: Projects::MergeRequestsController#show

Perform a Migration Analysis

• Client: GitLab-CE
• Route:
  • Projects::MergeRequestsController#show
  • Projects::MergeRequestsController#show.json

Expected Artefacts

See the Migration Process documentation for more information on the analysis stage of the process.

• Decision to move ahead with migration. Is it worth the effort right now?
• Existing reusable RPC endpoints. What have we already implemented?
• New RPC endpoints to be implemented. Just names. Leave the full design to the RPC Design stage next.
• Effort - for each new and existing RPC endpoint, roughly (small/medium/large) how much effort will be required to implement it?

assigned to @eReGeBe

changed the description

mentioned in issue #263 (closed)

It looks like this change might have affected this endpoint: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11518

I'm investigating a bit further....

@smcgivern this endpoint seems to be getting hit a lot as part of some polling operation.

It ranks consistantly highly in our prioritisation list but it seems that it might be a better candidate for ETag caching rather than Gitaly migration as it's the number of times the endpoint is hit, rather than it's git timing numbers that causing it to rank highly.

Is my assumption correct? And if so, do you agree with my course of action? Finally, if so, what's the best way of getting ETag caching prioritised?

To hit that point home, the most costly git operation takes 0.14%

@smcgivern is already digging into it, and it seems some DB migrations could go a long way https://gitlab.com/gitlab-com/gitlab-profiler/merge_requests/12#note_32097685

As for Gitaly, I think we should move on to the next item on the prioritization link (btw, your link doesn't load anything at the moment @andrewn)

It's interesting that this does in fact hit git (when I thought it didn't) - it's when we serialise the pipeline, and the commit is presented as part of that.

@andrewn we do have an issue to use ETag caching here, but the returned resource can be invalidated in a lot of places: https://gitlab.com/gitlab-org/gitlab-ce/issues/32545

We should fix the two other problems anyway: https://gitlab.com/gitlab-org/gitlab-ce/issues/33635 / https://gitlab.com/gitlab-org/gitlab-ce/issues/30224

I don't think there's anything for the Gitaly team to do here?

Thanks for weighing in on this @smcgivern. We'll move on for now :)

As an aside, we probably need to tune our prioritisation algorithm to avoid high-frequency routes like this from dominating our results.

@andrewn is there any way you can weight specific endpoints down? This one, while it does hit git, is completely dominated by other issues. Although having said that, the method call wall times in https://performance.gitlab.net/dashboard/db/rails-controllers?from=now-3h&to=now&orgId=1&var-action=Projects::MergeRequestsController%23show.json&var-database=Production do show a lot of time in git in the worst cases:

Maybe we need a different profile for those Although we should only call fetch_ref once for old MRs, and never for newer MRs, so I wouldn't be too worried about that either.

changed the description

removed assignee

closed