diff options
author | Shinya Maeda <shinya@gitlab.com> | 2019-07-18 12:22:46 +0300 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2019-07-23 13:26:08 +0300 |
commit | c2e0e689f355555db231ac6db40ab1b654c90233 (patch) | |
tree | 1ab8b2e6561598a61a10a0197a975019ed13f464 /app | |
parent | 1a3fda63a5f9756cde19bc7e221651b0c33cb5dc (diff) |
Validate the existence of archived traces before removing live trace
Often live traces are removed even though the archived trace
doesn't exist. This commit checkes the existence strictly.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/ci/build.rb | 8 | ||||
-rw-r--r-- | app/models/ci/job_artifact.rb | 4 | ||||
-rw-r--r-- | app/services/ci/archive_trace_service.rb | 28 | ||||
-rw-r--r-- | app/workers/archive_trace_worker.rb | 2 | ||||
-rw-r--r-- | app/workers/ci/archive_traces_cron_worker.rb | 2 |
5 files changed, 38 insertions, 6 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 635fcc86166..da70cb9a9a7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -531,6 +531,14 @@ module Ci trace.exist? end + def has_live_trace? + trace.live_trace_exist? + end + + def has_archived_trace? + trace.archived_trace_exist? + end + def artifacts_file job_artifacts_archive&.file end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index f80e98e5bca..e132cb045e2 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -176,6 +176,10 @@ module Ci end end + def self.archived_trace_exists_for?(job_id) + where(job_id: job_id).trace.take&.file&.file&.exists? + end + private def file_format_adapter_class diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb index b5cfa1d019c..700d78361a4 100644 --- a/app/services/ci/archive_trace_service.rb +++ b/app/services/ci/archive_trace_service.rb @@ -2,8 +2,25 @@ module Ci class ArchiveTraceService - def execute(job) + def execute(job, worker_name:) + # TODO: Remove this logging once we confirmed new live trace architecture is functional. + # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667. + unless job.has_live_trace? + Sidekiq.logger.warn(class: worker_name, + message: 'The job does not have live trace but going to be archived.', + job_id: job.id) + return + end + job.trace.archive! + + # TODO: Remove this logging once we confirmed new live trace architecture is functional. + # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667. + unless job.has_archived_trace? + Sidekiq.logger.warn(class: worker_name, + message: 'The job does not have archived trace after archiving.', + job_id: job.id) + end rescue ::Gitlab::Ci::Trace::AlreadyArchivedError # It's already archived, thus we can safely ignore this exception. rescue => e @@ -11,7 +28,7 @@ module Ci # If `archive!` keeps failing for over a week, that could incur data loss. # (See more https://docs.gitlab.com/ee/administration/job_traces.html#new-live-trace-architecture) # In order to avoid interrupting the system, we do not raise an exception here. - archive_error(e, job) + archive_error(e, job, worker_name) end private @@ -22,9 +39,12 @@ module Ci "Counter of failed attempts of trace archiving") end - def archive_error(error, job) + def archive_error(error, job, worker_name) failed_archive_counter.increment - Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}" # rubocop:disable Gitlab/RailsLogger + + Sidekiq.logger.warn(class: worker_name, + message: "Failed to archive trace. message: #{error.message}.", + job_id: job.id) Gitlab::Sentry .track_exception(error, diff --git a/app/workers/archive_trace_worker.rb b/app/workers/archive_trace_worker.rb index 4a9becf0ca7..66f9b8d9e80 100644 --- a/app/workers/archive_trace_worker.rb +++ b/app/workers/archive_trace_worker.rb @@ -7,7 +7,7 @@ class ArchiveTraceWorker # rubocop: disable CodeReuse/ActiveRecord def perform(job_id) Ci::Build.without_archived_trace.find_by(id: job_id).try do |job| - Ci::ArchiveTraceService.new.execute(job) + Ci::ArchiveTraceService.new.execute(job, worker_name: self.class.name) end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb index f65ff239866..75e68d0233a 100644 --- a/app/workers/ci/archive_traces_cron_worker.rb +++ b/app/workers/ci/archive_traces_cron_worker.rb @@ -11,7 +11,7 @@ module Ci # 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::ArchiveTraceService.new.execute(build) + Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name) end end # rubocop: enable CodeReuse/ActiveRecord |