Commit 226feebe authored by Furkan Ayhan's avatar Furkan Ayhan

Implement sending update frequency of build traces to runner

After this commit, runner will receive X-GitLab-Trace-Update-Interval
header in response of updating build trace.
When runner use this header for deciding update interval,
this will help to reduce requests to patch :id/trace endpoint.
parent d0caff41
...@@ -51,6 +51,8 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -51,6 +51,8 @@ class Projects::JobsController < Projects::ApplicationController
build.trace.read do |stream| build.trace.read do |stream|
respond_to do |format| respond_to do |format|
format.json do format.json do
build.trace.being_watched!
# TODO: when the feature flag is removed we should not pass # TODO: when the feature flag is removed we should not pass
# content_format to serialize method. # content_format to serialize method.
content_format = Feature.enabled?(:job_log_json, @project, default_enabled: true) ? :json : :html content_format = Feature.enabled?(:job_log_json, @project, default_enabled: true) ? :json : :html
......
---
title: Request less frequent updates from Runner when job log is not being watched
merge_request: 20841
author:
type: performance
...@@ -200,6 +200,10 @@ module API ...@@ -200,6 +200,10 @@ module API
status 202 status 202
header 'Job-Status', job.status header 'Job-Status', job.status
header 'Range', "0-#{stream_size}" header 'Range', "0-#{stream_size}"
if Feature.enabled?(:runner_job_trace_update_interval_header, default_enabled: true)
header 'X-GitLab-Trace-Update-Interval', job.trace.update_interval.to_s
end
end end
desc 'Authorize artifacts uploading for job' do desc 'Authorize artifacts uploading for job' do
......
...@@ -9,6 +9,10 @@ module Gitlab ...@@ -9,6 +9,10 @@ module Gitlab
LOCK_TTL = 10.minutes LOCK_TTL = 10.minutes
LOCK_RETRIES = 2 LOCK_RETRIES = 2
LOCK_SLEEP = 0.001.seconds LOCK_SLEEP = 0.001.seconds
WATCH_FLAG_TTL = 10.seconds
UPDATE_FREQUENCY_DEFAULT = 30.seconds
UPDATE_FREQUENCY_WHEN_BEING_WATCHED = 3.seconds
ArchiveError = Class.new(StandardError) ArchiveError = Class.new(StandardError)
AlreadyArchivedError = Class.new(StandardError) AlreadyArchivedError = Class.new(StandardError)
...@@ -119,6 +123,22 @@ module Gitlab ...@@ -119,6 +123,22 @@ module Gitlab
end end
end end
def update_interval
being_watched? ? UPDATE_FREQUENCY_WHEN_BEING_WATCHED : UPDATE_FREQUENCY_DEFAULT
end
def being_watched!
Gitlab::Redis::SharedState.with do |redis|
redis.set(being_watched_cache_key, true, ex: WATCH_FLAG_TTL)
end
end
def being_watched?
Gitlab::Redis::SharedState.with do |redis|
redis.exists(being_watched_cache_key)
end
end
private private
def unsafe_write!(mode, &blk) def unsafe_write!(mode, &blk)
...@@ -236,6 +256,10 @@ module Gitlab ...@@ -236,6 +256,10 @@ module Gitlab
def trace_artifact def trace_artifact
job.job_artifacts_trace job.job_artifacts_trace
end end
def being_watched_cache_key
"gitlab:ci:trace:#{job.id}:watched"
end
end end
end end
end end
...@@ -556,6 +556,12 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -556,6 +556,12 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
expect(json_response['status']).to eq job.status expect(json_response['status']).to eq job.status
expect(json_response['lines']).to eq [{ 'content' => [{ 'text' => 'BUILD TRACE' }], 'offset' => 0 }] expect(json_response['lines']).to eq [{ 'content' => [{ 'text' => 'BUILD TRACE' }], 'offset' => 0 }]
end end
it 'sets being-watched flag for the job' do
expect(response).to have_gitlab_http_status(:ok)
expect(job.trace.being_watched?).to be(true)
end
end end
context 'when job has no traces' do context 'when job has no traces' do
......
...@@ -26,4 +26,66 @@ describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do ...@@ -26,4 +26,66 @@ describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do
it_behaves_like 'trace with enabled live trace feature' it_behaves_like 'trace with enabled live trace feature'
end end
describe '#update_interval' do
context 'it is not being watched' do
it 'returns 30 seconds' do
expect(trace.update_interval).to eq(30.seconds)
end
end
context 'it is being watched' do
before do
trace.being_watched!
end
it 'returns 3 seconds' do
expect(trace.update_interval).to eq(3.seconds)
end
end
end
describe '#being_watched!' do
let(:cache_key) { "gitlab:ci:trace:#{build.id}:watched" }
it 'sets gitlab:ci:trace:<job.id>:watched in redis' do
trace.being_watched!
result = Gitlab::Redis::SharedState.with do |redis|
redis.exists(cache_key)
end
expect(result).to eq(true)
end
it 'updates the expiry of gitlab:ci:trace:<job.id>:watched in redis', :clean_gitlab_redis_shared_state do
Gitlab::Redis::SharedState.with do |redis|
redis.set(cache_key, true, ex: 4.seconds)
end
expect do
trace.being_watched!
end.to change { Gitlab::Redis::SharedState.with { |redis| redis.pttl(cache_key) } }
end
end
describe '#being_watched?' do
context 'gitlab:ci:trace:<job.id>:watched in redis is set', :clean_gitlab_redis_shared_state do
before do
Gitlab::Redis::SharedState.with do |redis|
redis.set("gitlab:ci:trace:#{build.id}:watched", true)
end
end
it 'returns true' do
expect(trace.being_watched?).to be(true)
end
end
context 'gitlab:ci:trace:<job.id>:watched in redis is not set' do
it 'returns false' do
expect(trace.being_watched?).to be(false)
end
end
end
end end
...@@ -1154,6 +1154,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -1154,6 +1154,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
expect(response.header).to have_key 'Range' expect(response.header).to have_key 'Range'
expect(response.header).to have_key 'Job-Status' expect(response.header).to have_key 'Job-Status'
expect(response.header).to have_key 'X-GitLab-Trace-Update-Interval'
end end
context 'when job has been updated recently' do context 'when job has been updated recently' do
...@@ -1291,6 +1292,41 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -1291,6 +1292,41 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
expect(response.header['Job-Status']).to eq 'canceled' expect(response.header['Job-Status']).to eq 'canceled'
end end
end end
context 'when build trace is being watched' do
before do
job.trace.being_watched!
end
it 'returns X-GitLab-Trace-Update-Interval as 3' do
patch_the_trace
expect(response.status).to eq 202
expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('3')
end
end
context 'when build trace is not being watched' do
it 'returns X-GitLab-Trace-Update-Interval as 30' do
patch_the_trace
expect(response.status).to eq 202
expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('30')
end
end
context 'when feature flag runner_job_trace_update_interval_header is disabled' do
before do
stub_feature_flags(runner_job_trace_update_interval_header: { enabled: false })
end
it 'does not return X-GitLab-Trace-Update-Interval header' do
patch_the_trace
expect(response.status).to eq 202
expect(response.header).not_to have_key 'X-GitLab-Trace-Update-Interval'
end
end
end end
context 'when Runner makes a force-patch' do context 'when Runner makes a force-patch' do
......
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