diff --git a/app/assets/javascripts/network/branch_graph.js b/app/assets/javascripts/network/branch_graph.js index 54fe9d19002..71894b4ff3e 100644 --- a/app/assets/javascripts/network/branch_graph.js +++ b/app/assets/javascripts/network/branch_graph.js @@ -98,6 +98,7 @@ export default class BranchGraph { let len = 0; let cuday = 0; let cumonth = ''; + let cuyear = ''; const { r } = this; r.rect(0, 0, 40, this.barHeight).attr({ fill: '#222', @@ -108,24 +109,21 @@ export default class BranchGraph { const ref = this.days; for (mm = 0, len = ref.length; mm < len; mm += 1) { const day = ref[mm]; - if (cuday !== day[0] || cumonth !== day[1]) { + if (cuday !== day[0] || cumonth !== day[1] || cuyear !== day[2]) { // Dates r.text(55, this.offsetY + this.unitTime * mm, day[0]).attr({ font: '12px Monaco, monospace', fill: '#BBB', }); - [cuday] = day; } - if (cumonth !== day[1]) { + if (cumonth !== day[1] || cuyear !== day[2]) { // Months r.text(20, this.offsetY + this.unitTime * mm, day[1]).attr({ font: '12px Monaco, monospace', fill: '#EEE', }); - - // eslint-disable-next-line prefer-destructuring - cumonth = day[1]; } + [cuday, cumonth, cuyear] = day; } this.renderPartialGraph(); return this.bindEvents(); diff --git a/app/views/projects/network/show.json.erb b/app/views/projects/network/show.json.erb index a146d137c55..93b3c9911e2 100644 --- a/app/views/projects/network/show.json.erb +++ b/app/views/projects/network/show.json.erb @@ -2,7 +2,7 @@ <%= raw( { - days: @graph.days.compact.map { |d| [d.day, d.strftime("%b")] }, + days: @graph.days.compact.map { |d| [d.day, d.strftime("%b"), d.year] }, commits: @graph.commits.map do |c| { parents: parents_zip_spaces(c.parents(@graph.map), c.parent_spaces), diff --git a/app/workers/concerns/application_worker.rb b/app/workers/concerns/application_worker.rb index 03a0b5fae00..d0b09c15289 100644 --- a/app/workers/concerns/application_worker.rb +++ b/app/workers/concerns/application_worker.rb @@ -93,9 +93,11 @@ module ApplicationWorker end def perform_async(*args) + return super if Gitlab::Database::LoadBalancing.primary_only? + # Worker execution for workers with data_consistency set to :delayed or :sticky # will be delayed to give replication enough time to complete - if utilizes_load_balancing_capabilities? + if utilizes_load_balancing_capabilities? && Feature.disabled?(:skip_scheduling_workers_for_replicas, default_enabled: :yaml) perform_in(delay_interval, *args) else super diff --git a/config/feature_flags/development/skip_scheduling_workers_for_replicas.yml b/config/feature_flags/development/skip_scheduling_workers_for_replicas.yml new file mode 100644 index 00000000000..494bec1e665 --- /dev/null +++ b/config/feature_flags/development/skip_scheduling_workers_for_replicas.yml @@ -0,0 +1,8 @@ +--- +name: skip_scheduling_workers_for_replicas +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74532 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1380 +milestone: '14.5' +type: development +group: group::project management +default_enabled: false diff --git a/doc/administration/geo/secondary_proxy/index.md b/doc/administration/geo/secondary_proxy/index.md index b0d0861066a..2b8c0d1e6fa 100644 --- a/doc/administration/geo/secondary_proxy/index.md +++ b/doc/administration/geo/secondary_proxy/index.md @@ -7,14 +7,11 @@ type: howto # Geo proxying for secondary sites **(PREMIUM SELF)** -> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. -> - [Enabled by default for unified URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.5. -> - [Disabled by default for different URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.5 [with a flag](../../feature_flags.md) named `geo_secondary_proxy_separate_urls`. +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. FLAG: -On self-managed GitLab, this feature is only available by default for Geo sites using a unified URL. See below to -[set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). -The feature is not ready for production use with separate URLs. +On self-managed GitLab, by default this feature is not available. See below to [Set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). +The feature is not ready for production use. Use Geo proxying to: @@ -68,10 +65,8 @@ a single URL used by all Geo sites, including the primary. In the Geo administration page of the **primary** site, edit each Geo secondary that is using the secondary proxying and set the `URL` field to the single URL. Make sure the primary site is also using this URL. - -## Disable Geo proxying -You can disable the secondary proxying on each Geo site, separately, by following these steps: +### Enable secondary proxying 1. SSH into each application node (serving user traffic directly) on your secondary Geo site and add the following environment variable: @@ -82,7 +77,7 @@ You can disable the secondary proxying on each Geo site, separately, by followin ```ruby gitlab_workhorse['env'] = { - "GEO_SECONDARY_PROXY" => "0" + "GEO_SECONDARY_PROXY" => "1" } ``` @@ -92,34 +87,18 @@ You can disable the secondary proxying on each Geo site, separately, by followin gitlab-ctl reconfigure ``` +1. SSH into one node running Rails on your primary Geo site and enable the Geo secondary proxy feature flag: + + ```shell + sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy)" + ``` + ## Enable Geo proxying with Separate URLs The ability to use proxying with separate URLs is still in development. You can follow the ["Geo secondary proxying with separate URLs" epic](https://gitlab.com/groups/gitlab-org/-/epics/6865) for progress. -To try out this feature, enable the `geo_secondary_proxy_separate_urls` feature flag. -SSH into one node running Rails on your primary Geo site and run: - -```shell -sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy_separate_urls)" -``` - -## Limitations - -The asynchronous Geo replication can cause unexpected issues when secondary proxying is used, for accelerated -data types that may be replicated to the Geo secondaries with a delay. - -For example, we found a potential issue where -[Replication lag introduces read-your-own-write inconsistencies](https://gitlab.com/gitlab-org/gitlab/-/issues/345267). -If the replication lag is high enough, this can result in Git reads receiving stale data when hitting a secondary. - -Non-Rails requests are not proxied, so other services may need to use a separate, non-unified URL to ensure requests -are always sent to the primary. These services include: - -- GitLab Container Registry - [can be configured to use a separate domain](../../packages/container_registry.md#configure-container-registry-under-its-own-domain). -- GitLab Pages - should always use a separate domain, as part of [the prerequisites for running GitLab Pages](../../pages/index.md#prerequisites). - ## Features accelerated by secondary Geo sites Most HTTP traffic sent to a secondary Geo site can be proxied to the primary Geo site. With this architecture, diff --git a/doc/administration/geo/setup/index.md b/doc/administration/geo/setup/index.md index 7d365f73101..84dff69ebe7 100644 --- a/doc/administration/geo/setup/index.md +++ b/doc/administration/geo/setup/index.md @@ -26,7 +26,6 @@ If you installed GitLab using the Omnibus packages (highly recommended): 1. [Configure GitLab](../replication/configuration.md) to set the **primary** and **secondary** site(s). 1. Optional: [Configure a secondary LDAP server](../../auth/ldap/index.md) for the **secondary** site(s). See [notes on LDAP](../index.md#ldap). 1. Follow the [Using a Geo Site](../replication/usage.md) guide. -1. [Configure Geo secondary proxying](../secondary_proxy/index.md) to use a single, unified URL for all Geo sites. This step is recommended to accelerate most read requests while transparently proxying writes to the primary Geo site. ## Post-installation documentation diff --git a/doc/integration/ding_talk.md b/doc/integration/ding_talk.md index 25381926fbf..2bc4f29298a 100644 --- a/doc/integration/ding_talk.md +++ b/doc/integration/ding_talk.md @@ -60,7 +60,7 @@ Sign in to DingTalk Open Platform and create an application on it. DingTalk gene ```ruby gitlab_rails['omniauth_providers'] = [ { - name: "ding_talk", + name: "dingtalk", # label: "Provider name", # optional label for login button, defaults to "Ding Talk" app_id: "YOUR_APP_ID", app_secret: "YOUR_APP_SECRET" @@ -71,7 +71,7 @@ Sign in to DingTalk Open Platform and create an application on it. DingTalk gene For installations from source: ```yaml - - { name: 'ding_talk', + - { name: 'dingtalk', # label: 'Provider name', # optional label for login button, defaults to "Ding Talk" app_id: 'YOUR_APP_ID', app_secret: 'YOUR_APP_SECRET' } diff --git a/doc/user/admin_area/analytics/usage_trends.md b/doc/user/admin_area/analytics/usage_trends.md index 6b0abd79c93..7901d30c3ea 100644 --- a/doc/user/admin_area/analytics/usage_trends.md +++ b/doc/user/admin_area/analytics/usage_trends.md @@ -18,7 +18,9 @@ This feature might not be available to you. Check the **version history** note a Usage Trends gives you an overview of how much data your instance contains, and how quickly this volume is changing over time. Usage Trends data refreshes daily. -To see Usage Trends: +## View Usage Trends + +To view Usage Trends: 1. On the top bar, select **Menu > Admin**. 1. On the left sidebar, select **Analytics > Usage Trends**. diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 899e2e6c1c5..242979da367 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -19,7 +19,8 @@ module Gitlab end def self.refmap - [:heads, :tags, '+refs/pull-requests/*/to:refs/merge-requests/*/head'] + # We omit :heads and :tags since these are fetched in the import_repository + ['+refs/pull-requests/*/to:refs/merge-requests/*/head'] end # Unlike GitHub, you can't grab the commit SHAs for pull requests that @@ -140,11 +141,11 @@ module Gitlab def import_repository log_info(stage: 'import_repository', message: 'starting import') - project.ensure_repository + project.repository.import_repository(project.import_url) project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap) log_info(stage: 'import_repository', message: 'finished import') - rescue Gitlab::Shell::Error => e + rescue ::Gitlab::Git::CommandError => e Gitlab::ErrorTracking.log_exception( e, stage: 'import_repository', message: 'failed import', error: e.message diff --git a/lib/gitlab/daemon.rb b/lib/gitlab/daemon.rb index ddb9d907640..058fe1c8139 100644 --- a/lib/gitlab/daemon.rb +++ b/lib/gitlab/daemon.rb @@ -2,16 +2,16 @@ module Gitlab class Daemon - def self.initialize_instance(*args) + def self.initialize_instance(...) raise "#{name} singleton instance already initialized" if @instance - @instance = new(*args) + @instance = new(...) Kernel.at_exit(&@instance.method(:stop)) @instance end - def self.instance(*args) - @instance ||= initialize_instance(*args) + def self.instance(...) + @instance ||= initialize_instance(...) end attr_reader :thread @@ -20,7 +20,8 @@ module Gitlab !thread.nil? end - def initialize + def initialize(**options) + @synchronous = options[:synchronous] @mutex = Mutex.new end @@ -43,6 +44,10 @@ module Gitlab Thread.current.name = thread_name run_thread end + + @thread.join if @synchronous + + @thread end end end diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index 52eb0764ae3..e16db5af8ce 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -30,6 +30,10 @@ module Gitlab end end + def self.primary_only? + each_load_balancer.all?(&:primary_only?) + end + def self.release_hosts each_load_balancer(&:release_host) end diff --git a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb index b9acc36b4cc..f0fd645a421 100644 --- a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb +++ b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb @@ -6,6 +6,8 @@ module Gitlab class SidekiqServerMiddleware JobReplicaNotUpToDate = Class.new(StandardError) + MINIMUM_DELAY_INTERVAL = 1 + def call(worker, job, _queue) worker_class = worker.class strategy = select_load_balancing_strategy(worker_class, job) @@ -42,7 +44,9 @@ module Gitlab wal_locations = get_wal_locations(job) - return :primary_no_wal unless wal_locations + return :primary_no_wal if wal_locations.blank? + + sleep_if_needed(job) if databases_in_sync?(wal_locations) # Happy case: we can read from a replica. @@ -56,6 +60,11 @@ module Gitlab end end + def sleep_if_needed(job) + time_diff = Time.current.to_f - job['created_at'].to_f + sleep time_diff if time_diff > 0 && time_diff < MINIMUM_DELAY_INTERVAL + end + def get_wal_locations(job) job['dedup_wal_locations'] || job['wal_locations'] || legacy_wal_location(job) end diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index f9313f0ff28..0380ddd9a2e 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -27,20 +27,26 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do end describe '#import_repository' do + let(:repo_url) { 'http://bitbucket:test@my-bitbucket' } + + before do + expect(project.repository).to receive(:import_repository).with(repo_url) + end + it 'adds a remote' do expect(subject).to receive(:import_pull_requests) expect(subject).to receive(:delete_temp_branches) expect(project.repository).to receive(:fetch_as_mirror) - .with('http://bitbucket:test@my-bitbucket', - refmap: [:heads, :tags, '+refs/pull-requests/*/to:refs/merge-requests/*/head']) + .with(repo_url, + refmap: ['+refs/pull-requests/*/to:refs/merge-requests/*/head']) subject.execute end - it 'raises a Gitlab::Shell exception in the fetch' do - expect(project.repository).to receive(:fetch_as_mirror).and_raise(Gitlab::Shell::Error) + it 'raises a Gitlab::Git::CommandError in the fetch' do + expect(project.repository).to receive(:fetch_as_mirror).and_raise(::Gitlab::Git::CommandError) - expect { subject.execute }.to raise_error(Gitlab::Shell::Error) + expect { subject.execute }.to raise_error(::Gitlab::Git::CommandError) end it 'raises an unhandled exception in the fetch' do diff --git a/spec/lib/gitlab/daemon_spec.rb b/spec/lib/gitlab/daemon_spec.rb index 075a1e414c7..4d11b0bdc6c 100644 --- a/spec/lib/gitlab/daemon_spec.rb +++ b/spec/lib/gitlab/daemon_spec.rb @@ -46,6 +46,30 @@ RSpec.describe Gitlab::Daemon do expect(subject).to have_received(:run_thread) end + + context '@synchronous' do + context 'when @synchronous is set to true' do + subject { described_class.instance(synchronous: true) } + + it 'calls join on the thread' do + # Thread has to be run in a block, expect_next_instance_of does not support this. + expect_any_instance_of(Thread).to receive(:join) # rubocop:disable RSpec/AnyInstanceOf + + subject.start + end + end + + context 'when @synchronous is not set to a truthy value' do + subject { described_class.instance } + + it 'does not call join on the thread' do + # Thread has to be run in a block, expect_next_instance_of does not support this. + expect_any_instance_of(Thread).not_to receive(:join) # rubocop:disable RSpec/AnyInstanceOf + + subject.start + end + end + end end describe '#stop' do diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index de2ad662d16..1c02fd0c1f8 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -77,9 +77,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ include_examples 'load balancing strategy', expected_strategy end - shared_examples_for 'sticks based on data consistency' do |data_consistency| - include_context 'data consistency worker class', data_consistency, :load_balancing_for_test_data_consistency_worker - + shared_examples_for 'sticks based on data consistency' do context 'when load_balancing_for_test_data_consistency_worker is disabled' do before do stub_feature_flags(load_balancing_for_test_data_consistency_worker: false) @@ -136,6 +134,52 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ end end + shared_examples_for 'sleeps when necessary' do + context 'when WAL locations are blank', :freeze_time do + let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", "wal_locations" => {}, "created_at" => Time.current.to_f - (described_class::MINIMUM_DELAY_INTERVAL - 0.3) } } + + it 'does not sleep' do + expect(middleware).not_to receive(:sleep) + + run_middleware + end + end + + context 'when WAL locations are present', :freeze_time do + let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", "database_replica_location" => "0/D525E3A8", "created_at" => Time.current.to_f - elapsed_time } } + + context 'when delay interval has not elapsed' do + let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL - 0.3 } + + it 'sleeps until the minimum delay is reached' do + expect(middleware).to receive(:sleep).with(be_within(0.01).of(elapsed_time)) + + run_middleware + end + end + + context 'when delay interval has elapsed' do + let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL + 0.3 } + + it 'does not sleep' do + expect(middleware).not_to receive(:sleep) + + run_middleware + end + end + + context 'when created_at is in the future' do + let(:elapsed_time) { -5 } + + it 'does not sleep' do + expect(middleware).not_to receive(:sleep) + + run_middleware + end + end + end + end + context 'when worker class does not include ApplicationWorker' do let(:worker) { ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper.new } @@ -146,10 +190,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ include_context 'data consistency worker class', :always, :load_balancing_for_test_data_consistency_worker include_examples 'stick to the primary', 'primary' + + context 'when delay interval has not elapsed', :freeze_time do + let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'database_replica_location' => '0/D525E3A8', "created_at" => Time.current.to_f - elapsed_time } } + let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL - 0.3 } + + it 'does not sleep' do + expect(middleware).not_to receive(:sleep) + + run_middleware + end + end end context 'when worker data consistency is :delayed' do - include_examples 'sticks based on data consistency', :delayed + include_context 'data consistency worker class', :delayed, :load_balancing_for_test_data_consistency_worker + + include_examples 'sticks based on data consistency' + include_examples 'sleeps when necessary' context 'when replica is not up to date' do before do @@ -195,7 +253,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ end context 'when worker data consistency is :sticky' do - include_examples 'sticks based on data consistency', :sticky + include_context 'data consistency worker class', :sticky, :load_balancing_for_test_data_consistency_worker + + include_examples 'sticks based on data consistency' + include_examples 'sleeps when necessary' context 'when replica is not up to date' do before do @@ -255,7 +316,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ end def run_middleware - middleware.call(worker, job, double(:queue)) { yield } + middleware.call(worker, job, double(:queue)) { yield if block_given? } rescue described_class::JobReplicaNotUpToDate # we silence errors here that cause the job to retry end diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb index 65ffe539910..45878b2e266 100644 --- a/spec/lib/gitlab/database/load_balancing_spec.rb +++ b/spec/lib/gitlab/database/load_balancing_spec.rb @@ -38,6 +38,24 @@ RSpec.describe Gitlab::Database::LoadBalancing do end end + describe '.primary_only?' do + it 'returns true if all load balancers have no replicas' do + described_class.each_load_balancer do |lb| + allow(lb).to receive(:primary_only?).and_return(true) + end + + expect(described_class.primary_only?).to eq(true) + end + + it 'returns false if at least one has replicas' do + described_class.each_load_balancer.with_index do |lb, index| + allow(lb).to receive(:primary_only?).and_return(index != 0) + end + + expect(described_class.primary_only?).to eq(false) + end + end + describe '.release_hosts' do it 'releases the host of every load balancer' do described_class.each_load_balancer do |lb| diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index 914f5a30c3a..3fbd207c2e1 100644 --- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -239,6 +239,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do shared_context 'worker declaring data consistency' do let(:worker_class) { LBTestWorker } + let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => 'AB/12345' } } + let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", "wal_locations" => wal_locations } } before do stub_const('LBTestWorker', Class.new(TestWorker)) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5e85387ea14..b3ebda019ef 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -478,7 +478,3 @@ Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s # Initialize FactoryDefault to use create_default helper TestProf::FactoryDefault.init - -# Exclude the Geo proxy API request from getting on_next_request Warden handlers, -# necessary to prevent race conditions with feature tests not getting authenticated. -::Warden.asset_paths << %r{^/api/v4/geo/proxy$} diff --git a/spec/workers/build_hooks_worker_spec.rb b/spec/workers/build_hooks_worker_spec.rb index 5f7e7e5fb00..a69e188b441 100644 --- a/spec/workers/build_hooks_worker_spec.rb +++ b/spec/workers/build_hooks_worker_spec.rb @@ -23,14 +23,6 @@ RSpec.describe BuildHooksWorker do end end - describe '.perform_async' do - it 'delays scheduling a job by calling perform_in with default delay' do - expect(described_class).to receive(:perform_in).with(ApplicationWorker::DEFAULT_DELAY_INTERVAL.second, 123) - - described_class.perform_async(123) - end - end - it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed diff --git a/spec/workers/concerns/application_worker_spec.rb b/spec/workers/concerns/application_worker_spec.rb index fbf39b3c7cd..7608b5f49a1 100644 --- a/spec/workers/concerns/application_worker_spec.rb +++ b/spec/workers/concerns/application_worker_spec.rb @@ -248,41 +248,42 @@ RSpec.describe ApplicationWorker do end describe '.perform_async' do + using RSpec::Parameterized::TableSyntax + + where(:primary_only?, :skip_scheduling_ff, :data_consistency, :schedules_job?) do + true | false | :sticky | false + true | false | :delayed | false + true | false | :always | false + true | true | :sticky | false + true | true | :delayed | false + true | true | :always | false + false | false | :sticky | true + false | false | :delayed | true + false | false | :always | false + false | true | :sticky | false + false | true | :delayed | false + false | true | :always | false + end + before do stub_const(worker.name, worker) + worker.data_consistency(data_consistency) + + allow(Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(primary_only?) + stub_feature_flags(skip_scheduling_workers_for_replicas: skip_scheduling_ff) end - shared_examples_for 'worker utilizes load balancing capabilities' do |data_consistency| - before do - worker.data_consistency(data_consistency) - end - - it 'call perform_in' do - expect(worker).to receive(:perform_in).with(described_class::DEFAULT_DELAY_INTERVAL.seconds, 123) + with_them do + it 'schedules or enqueues the job correctly' do + if schedules_job? + expect(worker).to receive(:perform_in).with(described_class::DEFAULT_DELAY_INTERVAL.seconds, 123) + else + expect(worker).not_to receive(:perform_in) + end worker.perform_async(123) end end - - context 'when workers data consistency is :sticky' do - it_behaves_like 'worker utilizes load balancing capabilities', :sticky - end - - context 'when workers data consistency is :delayed' do - it_behaves_like 'worker utilizes load balancing capabilities', :delayed - end - - context 'when workers data consistency is :always' do - before do - worker.data_consistency(:always) - end - - it 'does not call perform_in' do - expect(worker).not_to receive(:perform_in) - - worker.perform_async - end - end end context 'different kinds of push_bulk' do diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index 6835569dfa8..e57f58d59dd 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -65,7 +65,7 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback Config: cfg, accessLogger: accessLogger, // Kind of a feature flag. See https://gitlab.com/groups/gitlab-org/-/epics/5914#note_564974130 - enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") != "0", + enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") == "1", geoProxyBackend: &url.URL{}, } if up.geoProxyPollSleep == nil { @@ -207,8 +207,8 @@ func (u *upstream) findGeoProxyRoute(cleanedPath string, r *http.Request) *route func (u *upstream) pollGeoProxyAPI() { for { - u.geoProxyPollSleep(geoProxyApiPollingInterval) u.callGeoProxyAPI() + u.geoProxyPollSleep(geoProxyApiPollingInterval) } } diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go index 4d8efed2b73..53c15bb7e91 100644 --- a/workhorse/internal/upstream/upstream_test.go +++ b/workhorse/internal/upstream/upstream_test.go @@ -310,9 +310,5 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h } } - // Since the first sleep happens before any API call, this ensures - // we call the API at least once. - waitForNextApiPoll() - return ws, ws.Close, waitForNextApiPoll }