diff --git a/config/feature_flags/experiment/stop_precalling_binary_for_blobs.yml b/config/feature_flags/experiment/stop_precalling_binary_for_blobs.yml new file mode 100644 index 0000000000000000000000000000000000000000..da81f318b2d030b6af2dcba99654466d3dc2fe93 --- /dev/null +++ b/config/feature_flags/experiment/stop_precalling_binary_for_blobs.yml @@ -0,0 +1,9 @@ +--- +name: stop_precalling_binary_for_blobs +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/424140 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142418 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17440 +milestone: '16.9' +group: group::code review +type: experiment +default_enabled: false diff --git a/lib/gitlab/gitaly_client/blobs_stitcher.rb b/lib/gitlab/gitaly_client/blobs_stitcher.rb index 95053207d1af3694c58fe2c2106390cd94d48ea8..af5bc3728f5e28a454147a12a6806590b673834e 100644 --- a/lib/gitlab/gitaly_client/blobs_stitcher.rb +++ b/lib/gitlab/gitaly_client/blobs_stitcher.rb @@ -33,6 +33,12 @@ def each def new_blob(blob_data) data = blob_data[:data_parts].join + binary = if Feature.enabled?(:stop_precalling_binary_for_blobs, type: :experiment) + {} + else + { binary: Gitlab::Git::Blob.binary?(data) } + end + Gitlab::Git::Blob.new( id: blob_data[:oid], mode: blob_data[:mode]&.to_s(8), @@ -41,7 +47,7 @@ def new_blob(blob_data) size: blob_data[:size], commit_id: blob_data[:revision], data: data, - binary: Gitlab::Git::Blob.binary?(data) + **binary ) end end diff --git a/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb b/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb index e0c3e8d4b402ec001c8d872713bbe29c65c95b51..1ab0e040c10a1fe2f8dca4195d5fc4d6b42d083b 100644 --- a/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb +++ b/spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb @@ -4,35 +4,78 @@ RSpec.describe Gitlab::GitalyClient::BlobsStitcher do describe 'enumeration' do - it 'combines segregated blob messages together' do - messages = [ - OpenStruct.new(oid: 'abcdef1', path: 'path/to/file', size: 1642, revision: 'f00ba7', mode: 0100644, data: "first-line\n"), - OpenStruct.new(oid: '', data: 'second-line'), - OpenStruct.new(oid: '', data: '', revision: 'f00ba7', path: 'path/to/non-existent/file'), - OpenStruct.new(oid: 'abcdef2', path: 'path/to/another-file', size: 2461, revision: 'f00ba8', mode: 0100644, data: "GIF87a\x90\x01".b) - ] - - blobs = described_class.new(messages).to_a - - expect(blobs.size).to be(2) - - expect(blobs[0].id).to eq('abcdef1') - expect(blobs[0].mode).to eq('100644') - expect(blobs[0].name).to eq('file') - expect(blobs[0].path).to eq('path/to/file') - expect(blobs[0].size).to eq(1642) - expect(blobs[0].commit_id).to eq('f00ba7') - expect(blobs[0].data).to eq("first-line\nsecond-line") - expect(blobs[0].binary_in_repo?).to be false - - expect(blobs[1].id).to eq('abcdef2') - expect(blobs[1].mode).to eq('100644') - expect(blobs[1].name).to eq('another-file') - expect(blobs[1].path).to eq('path/to/another-file') - expect(blobs[1].size).to eq(2461) - expect(blobs[1].commit_id).to eq('f00ba8') - expect(blobs[1].data).to eq("GIF87a\x90\x01".b) - expect(blobs[1].binary_in_repo?).to be true + context 'when increase_diff_file_performance is on' do + before do + stub_feature_flags(stop_precalling_binary_for_blobs: true) + end + + it 'combines segregated blob messages together' do + messages = [ + OpenStruct.new(oid: 'abcdef1', path: 'path/to/file', size: 1642, revision: 'f00ba7', mode: 0100644, data: "first-line\n"), + OpenStruct.new(oid: '', data: 'second-line'), + OpenStruct.new(oid: '', data: '', revision: 'f00ba7', path: 'path/to/non-existent/file'), + OpenStruct.new(oid: 'abcdef2', path: 'path/to/another-file', size: 2461, revision: 'f00ba8', mode: 0100644, data: "GIF87a\x90\x01".b) + ] + + blobs = described_class.new(messages).to_a + + expect(blobs.size).to be(2) + + expect(blobs[0].id).to eq('abcdef1') + expect(blobs[0].mode).to eq('100644') + expect(blobs[0].name).to eq('file') + expect(blobs[0].path).to eq('path/to/file') + expect(blobs[0].size).to eq(1642) + expect(blobs[0].commit_id).to eq('f00ba7') + expect(blobs[0].data).to eq("first-line\nsecond-line") + expect(blobs[0].binary_in_repo?).to be false + + expect(blobs[1].id).to eq('abcdef2') + expect(blobs[1].mode).to eq('100644') + expect(blobs[1].name).to eq('another-file') + expect(blobs[1].path).to eq('path/to/another-file') + expect(blobs[1].size).to eq(2461) + expect(blobs[1].commit_id).to eq('f00ba8') + expect(blobs[1].data).to eq("GIF87a\x90\x01".b) + expect(blobs[1].binary_in_repo?).to be true + end + end + + context 'when increase_diff_file_performance is off' do + before do + stub_feature_flags(stop_precalling_binary_for_blobs: false) + end + + it 'combines segregated blob messages together' do + messages = [ + OpenStruct.new(oid: 'abcdef1', path: 'path/to/file', size: 1642, revision: 'f00ba7', mode: 0100644, data: "first-line\n"), + OpenStruct.new(oid: '', data: 'second-line'), + OpenStruct.new(oid: '', data: '', revision: 'f00ba7', path: 'path/to/non-existent/file'), + OpenStruct.new(oid: 'abcdef2', path: 'path/to/another-file', size: 2461, revision: 'f00ba8', mode: 0100644, data: "GIF87a\x90\x01".b) + ] + + blobs = described_class.new(messages).to_a + + expect(blobs.size).to be(2) + + expect(blobs[0].id).to eq('abcdef1') + expect(blobs[0].mode).to eq('100644') + expect(blobs[0].name).to eq('file') + expect(blobs[0].path).to eq('path/to/file') + expect(blobs[0].size).to eq(1642) + expect(blobs[0].commit_id).to eq('f00ba7') + expect(blobs[0].data).to eq("first-line\nsecond-line") + expect(blobs[0].binary_in_repo?).to be false + + expect(blobs[1].id).to eq('abcdef2') + expect(blobs[1].mode).to eq('100644') + expect(blobs[1].name).to eq('another-file') + expect(blobs[1].path).to eq('path/to/another-file') + expect(blobs[1].size).to eq(2461) + expect(blobs[1].commit_id).to eq('f00ba8') + expect(blobs[1].data).to eq("GIF87a\x90\x01".b) + expect(blobs[1].binary_in_repo?).to be true + end end end end