diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-02-03 16:10:47 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-02-03 16:10:47 +0300 |
commit | 6fbe1c7ea612e438ca30cad7316724f341532e73 (patch) | |
tree | a2abf9632a711c3104831ba8db4624cbadaab435 | |
parent | d2ac8c77861b4bb16f8e372ee9bfc1aea284f5be (diff) | |
parent | 626471e79512bc9068c879d35801680e0db1ce72 (diff) |
Merge branch 'jv-too-many-branches' into 'master'
UpdateRemoteMirror: handle large number of branches
Closes #1878
See merge request gitlab-org/gitaly!1745
-rw-r--r-- | changelogs/unreleased/jv-too-many-branches.yml | 5 | ||||
-rw-r--r-- | ruby/lib/gitlab/git/gitlab_projects.rb | 37 | ||||
-rw-r--r-- | ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb | 39 |
3 files changed, 56 insertions, 25 deletions
diff --git a/changelogs/unreleased/jv-too-many-branches.yml b/changelogs/unreleased/jv-too-many-branches.yml new file mode 100644 index 000000000..3f4a0d004 --- /dev/null +++ b/changelogs/unreleased/jv-too-many-branches.yml @@ -0,0 +1,5 @@ +--- +title: 'UpdateRemoteMirror: handle large number of branches' +merge_request: 1745 +author: +type: fixed diff --git a/ruby/lib/gitlab/git/gitlab_projects.rb b/ruby/lib/gitlab/git/gitlab_projects.rb index bc3859fb4..a23cdfa91 100644 --- a/ruby/lib/gitlab/git/gitlab_projects.rb +++ b/ruby/lib/gitlab/git/gitlab_projects.rb @@ -6,6 +6,8 @@ module Gitlab include Gitlab::Git::Popen include Gitlab::Utils::StrongMemoize + BRANCHES_PER_PUSH = 10 + # Relative path is a directory name for repository with .git at the end. # Example: gitlab-org/gitlab-test.git attr_reader :repository_relative_path @@ -67,29 +69,36 @@ module Gitlab end def push_branches(remote_name, timeout, force, branch_names, env: {}) - logger.info "Pushing branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %W(#{Gitlab.config.git.bin_path} push) - cmd << '--force' if force - cmd += %W(-- #{remote_name}).concat(branch_names) + branch_names.each_slice(BRANCHES_PER_PUSH) do |branches| + logger.info "Pushing #{branches.count} branches from #{repository_absolute_path} to remote #{remote_name}" - success = run_with_timeout(cmd, timeout, repository_absolute_path, env) + cmd = %W(#{Gitlab.config.git.bin_path} push) + cmd << '--force' if force + cmd += %W(-- #{remote_name}).concat(branches) - logger.error("Pushing branches to remote #{remote_name} failed.") unless success + unless run_with_timeout(cmd, timeout, repository_absolute_path, env) + logger.error("Pushing branches to remote #{remote_name} failed.") + return false + end + end - success + true end def delete_remote_branches(remote_name, branch_names, env: {}) - branches = branch_names.map { |branch_name| ":#{branch_name}" } + branch_names.each_slice(BRANCHES_PER_PUSH) do |branches| + logger.info "Pushing #{branches.count} deleted branches from #{repository_absolute_path} to remote #{remote_name}" - logger.info "Pushing deleted branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}).concat(branches) + cmd = %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}) + branches.each { |branch| cmd << ":#{branch}" } - success = run(cmd, repository_absolute_path, env) - - logger.error("Pushing deleted branches to remote #{remote_name} failed.") unless success + unless run(cmd, repository_absolute_path, env) + logger.error("Pushing deleted branches to remote #{remote_name} failed.") + return false + end + end - success + true end protected diff --git a/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb b/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb index 85122ede6..a1ef8b51b 100644 --- a/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -56,27 +56,35 @@ describe Gitlab::Git::GitlabProjects do describe '#push_branches' do let(:remote_name) { 'remote-name' } - let(:branch_name) { 'master' } let(:env) { { 'GIT_SSH_COMMAND' => 'foo-command bar' } } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} push -- #{remote_name} #{branch_name}) } let(:force) { false } + let(:branch_names) { 20.times.map { |i| "branch#{i}" } } + let(:cmd1) do + %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}) + branch_names[0, 10] + end + let(:cmd2) do + %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}) + branch_names[10, 10] + end - subject { gl_projects.push_branches(remote_name, 600, force, [branch_name], env: env) } + subject { gl_projects.push_branches(remote_name, 600, force, branch_names, env: env) } it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, env, success: true) + stub_spawn(cmd1, 600, tmp_repo_path, env, success: true) + stub_spawn(cmd2, 600, tmp_repo_path, env, success: true) is_expected.to be_truthy end it 'fails' do - stub_spawn(cmd, 600, tmp_repo_path, env, success: false) + stub_spawn(cmd1, 600, tmp_repo_path, env, success: true) + stub_spawn(cmd2, 600, tmp_repo_path, env, success: false) is_expected.to be_falsy end context 'with --force' do - let(:cmd) { %W(#{Gitlab.config.git.bin_path} push --force -- #{remote_name} #{branch_name}) } + let(:branch_names) { ['master'] } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} push --force -- #{remote_name} #{branch_names[0]}) } let(:force) { true } it 'executes the command' do @@ -150,20 +158,29 @@ describe Gitlab::Git::GitlabProjects do describe '#delete_remote_branches' do let(:remote_name) { 'remote-name' } - let(:branch_name) { 'master' } + let(:branch_names) { 20.times.map { |i| "branch#{i}" } } let(:env) { { 'GIT_SSH_COMMAND' => 'foo-command bar' } } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} push -- #{remote_name} :#{branch_name}) } + let(:cmd1) do + %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}) + + branch_names[0, 10].map { |b| ':' + b } + end + let(:cmd2) do + %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}) + + branch_names[10, 10].map { |b| ':' + b } + end - subject { gl_projects.delete_remote_branches(remote_name, [branch_name], env: env) } + subject { gl_projects.delete_remote_branches(remote_name, branch_names, env: env) } it 'executes the command' do - stub_unlimited_spawn(cmd, tmp_repo_path, env, success: true) + stub_unlimited_spawn(cmd1, tmp_repo_path, env, success: true) + stub_unlimited_spawn(cmd2, tmp_repo_path, env, success: true) is_expected.to be_truthy end it 'fails' do - stub_unlimited_spawn(cmd, tmp_repo_path, env, success: false) + stub_unlimited_spawn(cmd1, tmp_repo_path, env, success: true) + stub_unlimited_spawn(cmd2, tmp_repo_path, env, success: false) is_expected.to be_falsy end |