diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml
index 1c745e6d9868973f8cfeceb4cc857c4395a41501..fbf335ac7777f587c56c38bff1a5f47b5007d346 100644
--- a/.gitlab/ci/rails.gitlab-ci.yml
+++ b/.gitlab/ci/rails.gitlab-ci.yml
@@ -24,8 +24,6 @@
.single-db-rspec:
extends: .single-db
- variables:
- GITLAB_USE_MODEL_LOAD_BALANCING: "false"
.rspec-base:
extends:
@@ -387,8 +385,6 @@ db:migrate-from-previous-major-version:
SETUP_DB: "false"
PROJECT_TO_CHECKOUT: "gitlab-foss"
TAG_TO_CHECKOUT: "v13.12.9"
- # FIXME: make this job work with `GITLAB_USE_MODEL_LOAD_BALANCING: true`, see https://gitlab.com/gitlab-org/gitlab/-/issues/355573
- GITLAB_USE_MODEL_LOAD_BALANCING: "false"
before_script:
- !reference [.default-before_script, before_script]
- '[[ -d "ee/" ]] || export PROJECT_TO_CHECKOUT="gitlab"'
diff --git a/config/feature_flags/development/use_model_load_balancing.yml b/config/feature_flags/development/use_model_load_balancing.yml
deleted file mode 100644
index 630e34acff39d619df8bfbbf13b73737a005ed23..0000000000000000000000000000000000000000
--- a/config/feature_flags/development/use_model_load_balancing.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: use_model_load_balancing
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73631
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344797
-milestone: '14.5'
-type: development
-group: group::sharding
-default_enabled: false
diff --git a/lib/gitlab/database/load_balancing/connection_proxy.rb b/lib/gitlab/database/load_balancing/connection_proxy.rb
index a91df2eccdd709a149a07715133fb484cb621df8..1be63da8896b167230f363c21980c0af4650d7af 100644
--- a/lib/gitlab/database/load_balancing/connection_proxy.rb
+++ b/lib/gitlab/database/load_balancing/connection_proxy.rb
@@ -13,13 +13,6 @@ class ConnectionProxy
WriteInsideReadOnlyTransactionError = Class.new(StandardError)
READ_ONLY_TRANSACTION_KEY = :load_balacing_read_only_transaction
- # The load balancer returned by connection might be different
- # between `model.connection.load_balancer` vs `model.load_balancer`
- #
- # The used `model.connection` is dependent on `use_model_load_balancing`.
- # See more in: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73949.
- #
- # Always use `model.load_balancer` or `model.sticking`.
attr_reader :load_balancer
# These methods perform writes after which we need to stick to the
diff --git a/lib/gitlab/database/load_balancing/setup.rb b/lib/gitlab/database/load_balancing/setup.rb
index 6d667e8ecf0a92e29ea961a931511a3de078f163..b97238253370b702938cd05f3b0bd0b6c3653144 100644
--- a/lib/gitlab/database/load_balancing/setup.rb
+++ b/lib/gitlab/database/load_balancing/setup.rb
@@ -17,7 +17,6 @@ def setup
configure_connection
setup_connection_proxy
setup_service_discovery
- setup_feature_flag_to_model_load_balancing
end
def configure_connection
@@ -45,21 +44,6 @@ def setup_connection_proxy
setup_class_attribute(:sticking, Sticking.new(load_balancer))
end
- # TODO: This is temporary code to gradually redirect traffic to use
- # a dedicated DB replicas, or DB primaries (depending on configuration)
- # This implements a sticky behavior for the current request if enabled.
- #
- # This is needed for Phase 3 and Phase 4 of application rollout
- # https://gitlab.com/groups/gitlab-org/-/epics/6160#progress
- #
- # If `GITLAB_USE_MODEL_LOAD_BALANCING` is set, its value is preferred
- # Otherwise, a `use_model_load_balancing` FF value is used
- def setup_feature_flag_to_model_load_balancing
- return if active_record_base?
-
- @model.singleton_class.prepend(ModelLoadBalancingFeatureFlagMixin)
- end
-
def setup_service_discovery
return unless configuration.service_discovery_enabled?
@@ -84,31 +68,6 @@ def setup_class_attribute(attribute, value)
def active_record_base?
@model == ActiveRecord::Base
end
-
- module ModelLoadBalancingFeatureFlagMixin
- extend ActiveSupport::Concern
-
- def use_model_load_balancing?
- # Cache environment variable and return env variable first if defined
- default_use_model_load_balancing_env = Gitlab.dev_or_test_env? || nil
- use_model_load_balancing_env = Gitlab::Utils.to_boolean(ENV.fetch('GITLAB_USE_MODEL_LOAD_BALANCING', default_use_model_load_balancing_env))
-
- unless use_model_load_balancing_env.nil?
- return use_model_load_balancing_env
- end
-
- # Check a feature flag using RequestStore (if active)
- return false unless Gitlab::SafeRequestStore.active?
-
- Gitlab::SafeRequestStore.fetch(:use_model_load_balancing) do
- Feature.enabled?(:use_model_load_balancing, default_enabled: :yaml)
- end
- end
-
- def connection
- use_model_load_balancing? ? super : ApplicationRecord.connection
- end
- end
end
end
end
diff --git a/spec/lib/gitlab/database/load_balancing/setup_spec.rb b/spec/lib/gitlab/database/load_balancing/setup_spec.rb
index 4d565ce137af60888f583a766a9dbbe363aa47db..c44637b8d06b5087c6e2e58ddbb7015f27a9eb97 100644
--- a/spec/lib/gitlab/database/load_balancing/setup_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/setup_spec.rb
@@ -10,7 +10,6 @@
expect(setup).to receive(:configure_connection)
expect(setup).to receive(:setup_connection_proxy)
expect(setup).to receive(:setup_service_discovery)
- expect(setup).to receive(:setup_feature_flag_to_model_load_balancing)
setup.setup
end
@@ -120,120 +119,46 @@
end
end
- describe '#setup_feature_flag_to_model_load_balancing', :reestablished_active_record_base do
+ context 'uses correct base models', :reestablished_active_record_base do
using RSpec::Parameterized::TableSyntax
where do
{
- "with model LB enabled it picks a dedicated CI connection" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: 'true',
+ "it picks a dedicated CI connection" => {
env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
request_store_active: false,
- ff_use_model_load_balancing: nil,
ff_force_no_sharing_primary_model: false,
expectations: {
main: { read: 'main_replica', write: 'main' },
ci: { read: 'ci_replica', write: 'ci' }
}
},
- "with model LB enabled and re-use of primary connection it uses CI connection for reads" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: 'true',
+ "with re-use of primary connection it uses CI connection for reads" => {
env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main',
request_store_active: false,
- ff_use_model_load_balancing: nil,
ff_force_no_sharing_primary_model: false,
expectations: {
main: { read: 'main_replica', write: 'main' },
ci: { read: 'ci_replica', write: 'main' }
}
},
- "with model LB disabled it fallbacks to use main" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: 'false',
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
- request_store_active: false,
- ff_use_model_load_balancing: nil,
- ff_force_no_sharing_primary_model: false,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'main_replica', write: 'main' }
- }
- },
- "with model LB disabled, but re-use configured it fallbacks to use main" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: 'false',
+ "with re-use and FF force_no_sharing_primary_model enabled with RequestStore it sticks FF and uses CI connection for reads and writes" => {
env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main',
- request_store_active: false,
- ff_use_model_load_balancing: nil,
- ff_force_no_sharing_primary_model: false,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'main_replica', write: 'main' }
- }
- },
- "with FF use_model_load_balancing disabled without RequestStore it uses main" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
- request_store_active: false,
- ff_use_model_load_balancing: false,
- ff_force_no_sharing_primary_model: false,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'main_replica', write: 'main' }
- }
- },
- "with FF use_model_load_balancing enabled without RequestStore sticking of FF does not work, so it fallbacks to use main" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
- request_store_active: false,
- ff_use_model_load_balancing: true,
- ff_force_no_sharing_primary_model: false,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'main_replica', write: 'main' }
- }
- },
- "with FF use_model_load_balancing disabled with RequestStore it uses main" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
- request_store_active: true,
- ff_use_model_load_balancing: false,
- ff_force_no_sharing_primary_model: false,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'main_replica', write: 'main' }
- }
- },
- "with FF use_model_load_balancing enabled with RequestStore it sticks FF and uses CI connection" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil,
request_store_active: true,
- ff_use_model_load_balancing: true,
- ff_force_no_sharing_primary_model: false,
+ ff_force_no_sharing_primary_model: true,
expectations: {
main: { read: 'main_replica', write: 'main' },
ci: { read: 'ci_replica', write: 'ci' }
}
},
- "with re-use and ff_use_model_load_balancing enabled and FF force_no_sharing_primary_model disabled with RequestStore it sticks FF and uses CI connection for reads" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
+ "with re-use and FF force_no_sharing_primary_model enabled without RequestStore it doesn't use FF and uses CI connection for reads only" => {
env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main',
request_store_active: true,
- ff_use_model_load_balancing: true,
ff_force_no_sharing_primary_model: false,
expectations: {
main: { read: 'main_replica', write: 'main' },
ci: { read: 'ci_replica', write: 'main' }
}
- },
- "with re-use and ff_use_model_load_balancing enabled and FF force_no_sharing_primary_model enabled with RequestStore it sticks FF and uses CI connection for reads" => {
- env_GITLAB_USE_MODEL_LOAD_BALANCING: nil,
- env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main',
- request_store_active: true,
- ff_use_model_load_balancing: true,
- ff_force_no_sharing_primary_model: true,
- expectations: {
- main: { read: 'main_replica', write: 'main' },
- ci: { read: 'ci_replica', write: 'ci' }
- }
}
}
end
@@ -285,9 +210,7 @@ def self.name
end
end
- stub_env('GITLAB_USE_MODEL_LOAD_BALANCING', env_GITLAB_USE_MODEL_LOAD_BALANCING)
stub_env('GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci', env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci)
- stub_feature_flags(use_model_load_balancing: ff_use_model_load_balancing)
# Make load balancer to force init with a dedicated replicas connections
models.each do |_, model|
diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb
index c58dba213ee007e90543e56416c020abe5834d69..83b71bc4b9d9783d3e6dd02b37049b38ddfed0bc 100644
--- a/spec/lib/gitlab/database_spec.rb
+++ b/spec/lib/gitlab/database_spec.rb
@@ -251,9 +251,6 @@
end
it 'does return a valid schema depending on a base model used', :request_store do
- # This is currently required as otherwise the `Ci::Build.connection` == `Project.connection`
- # ENV due to lib/gitlab/database/load_balancing/setup.rb:93
- stub_env('GITLAB_USE_MODEL_LOAD_BALANCING', '1')
# FF due to lib/gitlab/database/load_balancing/configuration.rb:92
stub_feature_flags(force_no_sharing_primary_model: true)