Commit fb65481d authored by Patrick Steinhardt's avatar Patrick Steinhardt

repository: Always use `ListBlobs()` to enumerate new blobs

In 0d5df6f0 (repository: Implement `#new_blobs` via `#list_blobs`,
2021-08-25), we have introduced an alternative implementation of
`#new_blobs` which uses `ListBlobs()` instead of `ListNewBlobs()`. While
there wasn't yet any change in behaviour, this conversion allows us to
convert `#new_blobs` to receive multiple new revisions as arguments such
that we can batch-load new blobs in a single call instead of having to
do so once per change.

Remove the feature flag that has been guarding this new implementation
such that we can continue with the conversion. While the new code has
only been tested in production for one day at the time of writing this
commit, there haven't been any issues. Furthermore, the risk of this
change was low from the beginning. So removing the feature flag this
quickly shouldn't be much of a concern.

Changelog: added
parent 11f7bc17
---
name: new_blobs_via_list_blobs
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68935
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339637
milestone: '14.3'
type: development
group: group::gitaly
default_enabled: false
......@@ -364,13 +364,7 @@ module Gitlab
return [] if newrev.blank? || newrev == ::Gitlab::Git::BLANK_SHA
strong_memoize("new_blobs_#{newrev}") do
if Feature.enabled?(:new_blobs_via_list_blobs)
blobs(['--not', '--all', '--not', newrev], with_paths: true, dynamic_timeout: dynamic_timeout)
else
wrapped_gitaly_errors do
gitaly_ref_client.list_new_blobs(newrev, REV_LIST_COMMIT_LIMIT, dynamic_timeout: dynamic_timeout)
end
end
blobs(['--not', '--all', '--not', newrev], with_paths: true, dynamic_timeout: dynamic_timeout)
end
end
......
......@@ -959,49 +959,22 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
subject { repository.new_blobs(new_commit).to_a }
context 'with :new_blobs_via_list_blobs enabled' do
before do
stub_feature_flags(new_blobs_via_list_blobs: true)
expect_next_instance_of(Gitlab::GitalyClient::BlobService) do |service|
expect(service)
.to receive(:list_blobs)
.with(['--not', '--all', '--not', new_commit],
limit: Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT,
with_paths: true,
dynamic_timeout: nil)
.and_call_original
end
end
it 'enumerates new blobs' do
expect(subject).to match_array(
[have_attributes(class: Gitlab::Git::Blob, id: new_blob, path: 'nested/new-blob.txt', size: 18)]
)
before do
expect_next_instance_of(Gitlab::GitalyClient::BlobService) do |service|
expect(service)
.to receive(:list_blobs)
.with(['--not', '--all', '--not', new_commit],
limit: Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT,
with_paths: true,
dynamic_timeout: nil)
.and_call_original
end
end
context 'with :new_blobs_via_list_blobs disabled' do
before do
stub_feature_flags(new_blobs_via_list_blobs: false)
expect_next_instance_of(Gitlab::GitalyClient::RefService) do |service|
expect(service)
.to receive(:list_new_blobs)
.with(new_commit,
Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT,
dynamic_timeout: nil)
.and_call_original
end
end
it 'enumerates new blobs' do
expect(subject).to match_array([Gitaly::NewBlobObject.new(
size: 18,
oid: new_blob,
path: "nested/new-blob.txt"
)])
end
it 'enumerates new blobs' do
expect(subject).to match_array(
[have_attributes(class: Gitlab::Git::Blob, id: new_blob, path: 'nested/new-blob.txt', size: 18)]
)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment