diff options
author | Stan Hu <stanhu@gmail.com> | 2019-06-19 10:44:04 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-06-19 12:04:24 -0700 |
commit | 30f52b690f3dc373f1ec98d9def9d9d2cd37832a (patch) | |
tree | db45c48023360ad8f9c294db8395d0c5b702efb0 | |
parent | 0d2537bfa942f4bf198241bfbc5c4083929f3e46 (diff) | |
download | gitlab-ce-30f52b690f3dc373f1ec98d9def9d9d2cd37832a.tar.gz |
Avoid storing backtraces from Bitbucket Cloud imports in the databasesh-clean-up-bitbucket-import-errors
We noticed in
https://gitlab.com/gitlab-com/gl-infra/production/issues/908 some
Bitbucket imports took over a second to load their projects row because
`import_error` was huge due to errors. To prevent this, we now:
1. Clean the backtraces
2. Log the details into importer.log
3. Omit the details from the database
-rw-r--r-- | changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/bitbucket_import/importer.rb | 28 | ||||
-rw-r--r-- | spec/lib/gitlab/bitbucket_import/importer_spec.rb | 27 |
3 files changed, 53 insertions, 7 deletions
diff --git a/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml b/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml new file mode 100644 index 00000000000..e4c9de74e6a --- /dev/null +++ b/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml @@ -0,0 +1,5 @@ +--- +title: Avoid storing backtraces from Bitbucket Cloud imports in the database +merge_request: 29862 +author: +type: performance diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index c9f0ed66a54..8047ef4fa15 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -11,6 +11,7 @@ module Gitlab { title: 'task', color: '#7F8C8D' }].freeze attr_reader :project, :client, :errors, :users + attr_accessor :logger def initialize(project) @project = project @@ -19,6 +20,7 @@ module Gitlab @labels = {} @errors = [] @users = {} + @logger = Gitlab::Import::Logger.build end def execute @@ -41,6 +43,18 @@ module Gitlab }.to_json) end + def store_pull_request_error(pull_request, ex) + backtrace = Gitlab::Profiler.clean_backtrace(ex.backtrace) + error = { type: :pull_request, iid: pull_request.iid, errors: ex.message, trace: backtrace, raw_response: pull_request.raw } + + log_error(error) + # Omit the details from the database to avoid blowing up usage in the error column + error.delete(:trace) + error.delete(:raw_response) + + errors << error + end + def gitlab_user_id(project, username) find_user_id(username) || project.creator_id end @@ -176,7 +190,7 @@ module Gitlab import_pull_request_comments(pull_request, merge_request) if merge_request.persisted? rescue StandardError => e - errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, trace: e.backtrace.join("\n"), raw_response: pull_request.raw } + store_pull_request_error(pull_request, e) end end @@ -254,6 +268,18 @@ module Gitlab updated_at: comment.updated_at } end + + def log_error(details) + logger.error(log_base_data.merge(details)) + end + + def log_base_data + { + class: self.class.name, + project_id: project.id, + project_path: project.full_path + } + end end end end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index 2e90f6c7f71..35700e0b588 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -98,12 +98,8 @@ describe Gitlab::BitbucketImport::Importer do describe '#import_pull_requests' do let(:source_branch_sha) { sample.commits.last } let(:target_branch_sha) { sample.commits.first } - - before do - allow(subject).to receive(:import_wiki) - allow(subject).to receive(:import_issues) - - pull_request = instance_double( + let(:pull_request) do + instance_double( Bitbucket::Representation::PullRequest, iid: 10, source_branch_sha: source_branch_sha, @@ -116,6 +112,11 @@ describe Gitlab::BitbucketImport::Importer do author: 'other', created_at: Time.now, updated_at: Time.now) + end + + before do + allow(subject).to receive(:import_wiki) + allow(subject).to receive(:import_issues) # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad @inline_note = instance_double( @@ -167,6 +168,20 @@ describe Gitlab::BitbucketImport::Importer do expect(reply_note.note).to eq(@reply.note) end + context 'when importing a pull request throws an exception' do + before do + allow(pull_request).to receive(:raw).and_return('hello world') + allow(subject.client).to receive(:pull_request_comments).and_raise(HTTParty::Error) + end + + it 'logs an error without the backtrace' do + subject.execute + + expect(subject.errors.count).to eq(1) + expect(subject.errors.first.keys).to match_array(%i(type iid errors)) + end + end + context "when branches' sha is not found in the repository" do let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH } let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH } |