Avoid Gitaly RPCs in rate-limited raw blob requests

As observed in
https://gitlab.com/gitlab-com/gl-infra/scalability/issues/77, a
rate-limited blob request would still cause disk I/O since
`assign_ref_vars` called FindCommit to look up the given commit.

To avoid this, we need to do a number of things:

1. Move rate-limiting to occur before the `assign_ref_vars` filter.

2. Call `extracts_path` directly to pull out the filename of the
request.

3. Use the path as the key for rate-limiting. This means that filenames
with different commits will now be rate-limited in the same bucket.
That should be okay since the intent of the rate-limiting is to block
abusive behavior, and requesting the same filename with different commit
IDs at a high rate could create performance issues.

4. Redirecting to another blob path would still cause Gitaly RPCs
because the controller would call `assign_ref_vars`. Now, just render a
plain text messsage with a 429 response code.

Closes https://gitlab.com/gitlab-com/gl-infra/scalability/issues/77

app/controllers/projects/raw_controller.rb

@@ -9,9 +9,9 @@ class Projects::RawController < Projects::ApplicationController
   prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:blob) }
 
   before_action :require_non_empty_project
-  before_action :assign_ref_vars
   before_action :authorize_download_code!
   before_action :show_rate_limit, only: [:show], unless: :external_storage_request?
+  before_action :assign_ref_vars
   before_action :redirect_to_external_storage, only: :show, if: :static_objects_external_storage_enabled?
 
   def show
@@ -23,11 +23,15 @@ class Projects::RawController < Projects::ApplicationController
   private
 
   def show_rate_limit
-    if rate_limiter.throttled?(:show_raw_controller, scope: [@project, @commit, @path], threshold: raw_blob_request_limit)
+    # This bypasses assign_ref_vars to avoid a Gitaly FindCommit lookup.
+    # When rate limiting, we really don't care if a different commit is
+    # being requested.
+    _ref, path = extract_ref(get_id)
+
+    if rate_limiter.throttled?(:show_raw_controller, scope: [@project, path], threshold: raw_blob_request_limit)
       rate_limiter.log_request(request, :raw_blob_request_limit, current_user)
 
-      flash[:alert] = _('You cannot access the raw file. Please wait a minute.')
-      redirect_to project_blob_path(@project, File.join(@ref, @path)), status: :too_many_requests
+      render plain: _('You cannot access the raw file. Please wait a minute.'), status: :too_many_requests
     end
   end
 

changelogs/unreleased/sh-skip-findcommit-lookup-rate-limit.yml

+---
+title: Avoid Gitaly RPCs in rate-limited raw blob requests
+merge_request: 22123
+author:
+type: performance

spec/controllers/projects/raw_controller_spec.rb

@@ -56,10 +56,13 @@ describe Projects::RawController do
         stub_application_setting(raw_blob_request_limit: 5)
       end
 
-      it 'prevents from accessing the raw file' do
-        execute_raw_requests(requests: 6, project: project, file_path: file_path)
+      it 'prevents from accessing the raw file', :request_store do
+        execute_raw_requests(requests: 5, project: project, file_path: file_path)
+
+        expect { execute_raw_requests(requests: 1, project: project, file_path: file_path) }
+          .to change { Gitlab::GitalyClient.get_request_count }.by(0)
 
-        expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.'))
+        expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
         expect(response).to have_gitlab_http_status(429)
       end
 
@@ -109,7 +112,7 @@ describe Projects::RawController do
 
           execute_raw_requests(requests: 3, project: project, file_path: modified_path)
 
-          expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.'))
+          expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
           expect(response).to have_gitlab_http_status(429)
         end
       end
@@ -137,7 +140,7 @@ describe Projects::RawController do
           # Accessing downcase version of readme
           execute_raw_requests(requests: 6, project: project, file_path: file_path)
 
-          expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.'))
+          expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
           expect(response).to have_gitlab_http_status(429)
 
           # Accessing upcase version of readme

spec/features/projects/raw/user_interacts_with_raw_endpoint_spec.rb

@@ -31,8 +31,6 @@ describe 'Projects > Raw > User interacts with raw endpoint' do
         visit project_raw_url(project, file_path)
       end
 
-      expect(source).to have_content('You are being redirected')
-      click_link('redirected')
       expect(page).to have_content('You cannot access the raw file. Please wait a minute.')
     end
   end