Skip to content

Add RequestCache to cache via RequestStore

username-removed-423915 requested to merge request-store-wrap into master

What does this MR do?

Add RequestCache to cache via RequestStore. This module makes it easy to cache methods in ReqeustStore.

Why was this MR needed?

I don't like the idea of RequestStore at all, because it's just a global state which shouldn't be used at all. But we have a number of places calling ProtectedBranch.protected? and ProtectedTag.protected? in a loop for the same user, project, and ref whenever we're checking against if the jobs for a given pipeline is accessible for a given user. This means we're effectively making N queries for the same thing over and over.

To properly fix this, we need to change how we check the permission, and that could be a huge work. To solve this quickly, adding a cache layer for the given request would be quite simple to do.

We're already doing this in Commit#author, and this is extending that idea and make it generalized.

Are there points in the code the reviewer needs to double check?

@stanhu @rymai I would like to know how you think about this. Also I would love to have a better name. I can't think of anything better at the moment.

/cc @grzesiek

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

This was extracted from !11910 (merged) in order to eliminate N+1 queries. I think we already have N queries for manual actions. It's just that in !11910 (merged) it would be going to be all jobs, not just manual actions, so the difference would be more.

Edited by username-removed-423915

Merge request reports