diff options
-rw-r--r-- | app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js | 3 | ||||
-rw-r--r-- | app/models/ci/build.rb | 2 | ||||
-rw-r--r-- | app/serializers/merge_request_widget_entity.rb | 4 | ||||
-rw-r--r-- | app/workers/ci/archive_traces_cron_worker.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/avoid-race-condition-of-archive-trace-cron-worker.yml | 5 | ||||
-rw-r--r-- | doc/user/gitlab_com/index.md | 16 | ||||
-rw-r--r-- | lib/gitlab/sidekiq_logging/structured_logger.rb | 46 | ||||
-rw-r--r-- | package.json | 2 | ||||
-rw-r--r-- | spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js | 42 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 50 | ||||
-rw-r--r-- | spec/workers/ci/archive_traces_cron_worker_spec.rb | 16 | ||||
-rw-r--r-- | yarn.lock | 8 |
13 files changed, 159 insertions, 74 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 0f55bebd3fc..7843409f4a7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -87,9 +87,6 @@ export default class MergeRequestStore { this.allowCollaboration = data.allow_collaboration; this.sourceProjectId = data.source_project_id; this.targetProjectId = data.target_project_id; - this.mergePipelinesEnabled = Boolean(data.merge_pipelines_enabled); - this.mergeTrainsCount = data.merge_trains_count || 0; - this.mergeTrainIndex = data.merge_train_index; // CI related this.hasCI = data.has_ci; diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 3c0efca31db..7930bef5cf2 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -121,6 +121,8 @@ module Ci scope :scheduled_actions, ->() { where(when: :delayed, status: COMPLETED_STATUSES + %i[scheduled]) } scope :ref_protected, -> { where(protected: true) } scope :with_live_trace, -> { where('EXISTS (?)', Ci::BuildTraceChunk.where('ci_builds.id = ci_build_trace_chunks.build_id').select(1)) } + scope :with_stale_live_trace, -> { with_live_trace.finished_before(12.hours.ago) } + scope :finished_before, -> (date) { finished.where('finished_at < ?', date) } scope :matches_tag_ids, -> (tag_ids) do matcher = ::ActsAsTaggableOn::Tagging diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 554b307d4f8..c8088608cb0 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -84,8 +84,10 @@ class MergeRequestWidgetEntity < Grape::Entity private + delegate :current_user, to: :request + def presenter(merge_request) @presenters ||= {} - @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: request.current_user) # rubocop: disable CodeReuse/Presenter + @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter end end diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb index 75e68d0233a..ef2da729705 100644 --- a/app/workers/ci/archive_traces_cron_worker.rb +++ b/app/workers/ci/archive_traces_cron_worker.rb @@ -10,7 +10,7 @@ module Ci # Archive stale live traces which still resides in redis or database # This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL # More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791 - Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build| + Ci::Build.with_stale_live_trace.find_each(batch_size: 100) do |build| Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name) end end diff --git a/changelogs/unreleased/avoid-race-condition-of-archive-trace-cron-worker.yml b/changelogs/unreleased/avoid-race-condition-of-archive-trace-cron-worker.yml new file mode 100644 index 00000000000..b3f84ec32d2 --- /dev/null +++ b/changelogs/unreleased/avoid-race-condition-of-archive-trace-cron-worker.yml @@ -0,0 +1,5 @@ +--- +title: Avoid conflicts between ArchiveTracesCronWorker and ArchiveTraceWorker +merge_request: 31376 +author: +type: fixed diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index d21a325d401..af37cc896ad 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -43,13 +43,15 @@ Host gitlab.com Below are the settings for [GitLab Pages]. -| Setting | GitLab.com | Default | -| ----------------------- | ---------------- | ------------- | -| Domain name | `gitlab.io` | - | -| IP address | `35.185.44.232` | - | -| Custom domains support | yes | no | -| TLS certificates support| yes | no | +| Setting | GitLab.com | Default | +| --------------------------- | ---------------- | ------------- | +| Domain name | `gitlab.io` | - | +| IP address | `35.185.44.232` | - | +| Custom domains support | yes | no | +| TLS certificates support | yes | no | +| Maximum size (uncompressed) | 1G | 100M | +NOTE: **Note:** The maximum size of your Pages site is regulated by the artifacts maximum size which is part of [GitLab CI/CD](#gitlab-cicd). @@ -59,7 +61,7 @@ Below are the current settings regarding [GitLab CI/CD](../../ci/README.md). | Setting | GitLab.com | Default | | ----------- | ----------------- | ------------- | -| Artifacts maximum size | 1G | 100M | +| Artifacts maximum size (uncompressed) | 1G | 100M | | Artifacts [expiry time](../../ci/yaml/README.md#artifactsexpire_in) | kept forever | deleted after 30 days unless otherwise specified | ## Repository size limit diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index 60782306ade..48b1524f9c7 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -8,16 +8,16 @@ module Gitlab MAXIMUM_JOB_ARGUMENTS_LENGTH = 10.kilobytes def call(job, queue) - started_at = current_time + started_time = get_time base_payload = parse_job(job) - Sidekiq.logger.info log_job_start(started_at, base_payload) + Sidekiq.logger.info log_job_start(base_payload) yield - Sidekiq.logger.info log_job_done(job, started_at, base_payload) + Sidekiq.logger.info log_job_done(job, started_time, base_payload) rescue => job_exception - Sidekiq.logger.warn log_job_done(job, started_at, base_payload, job_exception) + Sidekiq.logger.warn log_job_done(job, started_time, base_payload, job_exception) raise end @@ -32,7 +32,7 @@ module Gitlab output_payload.merge!(job.slice(*::Gitlab::InstrumentationHelper::KEYS)) end - def log_job_start(started_at, payload) + def log_job_start(payload) payload['message'] = "#{base_message(payload)}: start" payload['job_status'] = 'start' @@ -45,11 +45,12 @@ module Gitlab payload end - def log_job_done(job, started_at, payload, job_exception = nil) + def log_job_done(job, started_time, payload, job_exception = nil) payload = payload.dup add_instrumentation_keys!(job, payload) - payload['duration'] = elapsed(started_at) - payload['completed_at'] = Time.now.utc + + elapsed_time = elapsed(started_time) + add_time_keys!(elapsed_time, payload) message = base_message(payload) @@ -69,6 +70,14 @@ module Gitlab payload end + def add_time_keys!(time, payload) + payload['duration'] = time[:duration].round(3) + payload['system_s'] = time[:stime].round(3) + payload['user_s'] = time[:utime].round(3) + payload['child_s'] = time[:ctime].round(3) if time[:ctime] > 0 + payload['completed_at'] = Time.now.utc + end + def parse_job(job) job = job.dup @@ -93,8 +102,25 @@ module Gitlab (Time.now.utc - start).to_f.round(3) end - def elapsed(start) - (current_time - start).round(3) + def elapsed(t0) + t1 = get_time + { + duration: t1[:now] - t0[:now], + stime: t1[:times][:stime] - t0[:times][:stime], + utime: t1[:times][:utime] - t0[:times][:utime], + ctime: ctime(t1[:times]) - ctime(t0[:times]) + } + end + + def get_time + { + now: current_time, + times: Process.times + } + end + + def ctime(times) + times[:cstime] + times[:cutime] end def current_time diff --git a/package.json b/package.json index 97ea872ee61..df5f758fa20 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "@babel/preset-env": "^7.5.5", "@gitlab/csslab": "^1.9.0", "@gitlab/svgs": "^1.68.0", - "@gitlab/ui": "5.15.0", + "@gitlab/ui": "5.18.0", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", "apollo-link": "^1.2.11", diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js index e27a506f426..e2cd0f084fd 100644 --- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js +++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js @@ -82,47 +82,5 @@ describe('MergeRequestStore', () => { expect(store.isNothingToMergeState).toEqual(false); }); }); - - describe('mergePipelinesEnabled', () => { - it('should set mergePipelinesEnabled = true when merge_pipelines_enabled is true', () => { - store.setData({ ...mockData, merge_pipelines_enabled: true }); - - expect(store.mergePipelinesEnabled).toBe(true); - }); - - it('should set mergePipelinesEnabled = false when merge_pipelines_enabled is not provided', () => { - store.setData({ ...mockData, merge_pipelines_enabled: undefined }); - - expect(store.mergePipelinesEnabled).toBe(false); - }); - }); - - describe('mergeTrainsCount', () => { - it('should set mergeTrainsCount when merge_trains_count is provided', () => { - store.setData({ ...mockData, merge_trains_count: 3 }); - - expect(store.mergeTrainsCount).toBe(3); - }); - - it('should set mergeTrainsCount = 0 when merge_trains_count is not provided', () => { - store.setData({ ...mockData, merge_trains_count: undefined }); - - expect(store.mergeTrainsCount).toBe(0); - }); - }); - - describe('mergeTrainIndex', () => { - it('should set mergeTrainIndex when merge_train_index is provided', () => { - store.setData({ ...mockData, merge_train_index: 3 }); - - expect(store.mergeTrainIndex).toBe(3); - }); - - it('should not set mergeTrainIndex when merge_train_index is not provided', () => { - store.setData({ ...mockData, merge_train_index: undefined }); - - expect(store.mergeTrainIndex).toBeUndefined(); - }); - }); }); }); diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 5621d3d17d1..c09db4af8d8 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -36,7 +36,9 @@ describe Gitlab::SidekiqLogging::StructuredLogger do 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', 'job_status' => 'done', 'duration' => 0.0, - "completed_at" => timestamp.iso8601(3) + "completed_at" => timestamp.iso8601(3), + "system_s" => 0.0, + "user_s" => 0.0 ) end let(:exception_payload) do @@ -52,6 +54,13 @@ describe Gitlab::SidekiqLogging::StructuredLogger do allow(Sidekiq).to receive(:logger).and_return(logger) allow(subject).to receive(:current_time).and_return(timestamp.to_f) + + allow(Process).to receive(:times).and_return( + stime: 0.0, + utime: 0.0, + cutime: 0.0, + cstime: 0.0 + ) end subject { described_class.new } @@ -177,5 +186,31 @@ describe Gitlab::SidekiqLogging::StructuredLogger do end end end + + def ctime(times) + times[:cstime] + times[:cutime] + end + + context 'with ctime value greater than 0' do + let(:times_start) { { stime: 0.04999, utime: 0.0483, cstime: 0.0188, cutime: 0.0188 } } + let(:times_end) { { stime: 0.0699, utime: 0.0699, cstime: 0.0399, cutime: 0.0399 } } + + before do + end_payload['system_s'] = 0.02 + end_payload['user_s'] = 0.022 + end_payload['child_s'] = 0.042 + + allow(Process).to receive(:times).and_return(times_start, times_end) + end + + it 'logs with ctime data and other cpu data' do + Timecop.freeze(timestamp) do + expect(logger).to receive(:info).with(start_payload.except('args')).ordered + expect(logger).to receive(:info).with(end_payload.except('args')).ordered + + subject.call(job, 'test_queue') { } + end + end + end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 4aac4b640f4..bc853d45085 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -149,6 +149,56 @@ describe Ci::Build do end end + describe '.with_stale_live_trace' do + subject { described_class.with_stale_live_trace } + + context 'when build has a stale live trace' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build does not have a stale live trace' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.hour.ago) } + + it 'does not select the build' do + is_expected.to be_empty + end + end + end + + describe '.finished_before' do + subject { described_class.finished_before(date) } + + let(:date) { 1.hour.ago } + + context 'when build has finished one day ago' do + let!(:build) { create(:ci_build, :success, finished_at: 1.day.ago) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build has finished 30 minutes ago' do + let!(:build) { create(:ci_build, :success, finished_at: 30.minutes.ago) } + + it 'returns an empty array' do + is_expected.to be_empty + end + end + + context 'when build is still running' do + let!(:build) { create(:ci_build, :running) } + + it 'returns an empty array' do + is_expected.to be_empty + end + end + end + describe '.with_reports' do subject { described_class.with_reports(Ci::JobArtifact.test_reports) } diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index 28381fdc3be..01232e2a58b 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' describe Ci::ArchiveTracesCronWorker do subject { described_class.new.perform } + let(:finished_at) { 1.day.ago } + before do stub_feature_flags(ci_enable_live_trace: true) end @@ -28,7 +30,7 @@ describe Ci::ArchiveTracesCronWorker do end context 'when a job succeeded' do - let!(:build) { create(:ci_build, :success, :trace_live) } + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } it_behaves_like 'archives trace' @@ -39,9 +41,15 @@ describe Ci::ArchiveTracesCronWorker do subject end + context 'when the job finished recently' do + let(:finished_at) { 1.hour.ago } + + it_behaves_like 'does not archive trace' + end + context 'when a trace had already been archived' do let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) } - let!(:build2) { create(:ci_build, :success, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } it 'continues to archive live traces' do subject @@ -52,7 +60,7 @@ describe Ci::ArchiveTracesCronWorker do end context 'when an unexpected exception happened during archiving' do - let!(:build) { create(:ci_build, :success, :trace_live) } + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } before do allow(Gitlab::Sentry).to receive(:track_exception) @@ -71,7 +79,7 @@ describe Ci::ArchiveTracesCronWorker do end context 'when a job was cancelled' do - let!(:build) { create(:ci_build, :canceled, :trace_live) } + let!(:build) { create(:ci_build, :canceled, :trace_live, finished_at: finished_at) } it_behaves_like 'archives trace' end diff --git a/yarn.lock b/yarn.lock index 525c77e41d2..4489b10815a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1003,10 +1003,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.68.0.tgz#d631bd84ea7907592240d8417e82ba66d6a54c0c" integrity sha512-3JmIq0bHg4InjLooM+kQFPfg3d7B1Pye67pN9+12kZXIa0nRGuwKEq3WSbcS+ACdg5jcVPNPYqStItEO4teHdw== -"@gitlab/ui@5.15.0": - version "5.15.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.15.0.tgz#1ce92cfed77dcd63a90d751043b42b19e64431c9" - integrity sha512-YrYgERcmTxC+oP4GaY7onqvYgvTsyGCiiegQbZbXdNRLGGAmvfxWPzQRz8Ci9yIYkLvi0X2AV7BT8RTVOPQgXg== +"@gitlab/ui@5.18.0": + version "5.18.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.18.0.tgz#f7f93ad41673f5ae081cd2246dda5456d3c956a2" + integrity sha512-umB7JCKVDZOajed0N563JBYSpiyK+VDL2eCdkinW+twWhMct8onoW4CLTpkgBi6Z9Y1Bq47aqnFtGf+1IKNIGw== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" |