diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-20 16:18:24 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-20 16:18:24 +0300 |
| commit | 0653e08efd039a5905f3fa4f6e9cef9f5d2f799c (patch) | |
| tree | 4dcc884cf6d81db44adae4aa99f8ec1233a41f55 /lib/gitlab/database | |
| parent | 744144d28e3e7fddc117924fef88de5d9674fe4c (diff) | |
Add latest changes from gitlab-org/gitlab@14-3-stable-eev14.3.0-rc42
Diffstat (limited to 'lib/gitlab/database')
40 files changed, 908 insertions, 308 deletions
diff --git a/lib/gitlab/database/async_indexes/migration_helpers.rb b/lib/gitlab/database/async_indexes/migration_helpers.rb index dff6376270a..2f990aba2fb 100644 --- a/lib/gitlab/database/async_indexes/migration_helpers.rb +++ b/lib/gitlab/database/async_indexes/migration_helpers.rb @@ -55,11 +55,14 @@ module Gitlab schema_creation = ActiveRecord::ConnectionAdapters::PostgreSQL::SchemaCreation.new(ApplicationRecord.connection) definition = schema_creation.accept(create_index) - async_index = PostgresAsyncIndex.safe_find_or_create_by!(name: index_name) do |rec| + async_index = PostgresAsyncIndex.find_or_create_by!(name: index_name) do |rec| rec.table_name = table_name rec.definition = definition end + async_index.definition = definition + async_index.save! # No-op if definition is not changed + Gitlab::AppLogger.info( message: 'Prepared index for async creation', table_name: async_index.table_name, @@ -68,8 +71,6 @@ module Gitlab async_index end - private - def async_index_creation_available? ApplicationRecord.connection.table_exists?(:postgres_async_indexes) && Feature.enabled?(:database_async_index_creation, type: :ops) diff --git a/lib/gitlab/database/background_migration/batched_job.rb b/lib/gitlab/database/background_migration/batched_job.rb index 03bd02d7554..32765cb6a56 100644 --- a/lib/gitlab/database/background_migration/batched_job.rb +++ b/lib/gitlab/database/background_migration/batched_job.rb @@ -4,6 +4,7 @@ module Gitlab module Database module BackgroundMigration class BatchedJob < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord + include EachBatch include FromUnion self.table_name = :batched_background_migration_jobs diff --git a/lib/gitlab/database/background_migration/batched_migration.rb b/lib/gitlab/database/background_migration/batched_migration.rb index 9d66824da51..d9fc2ea48f6 100644 --- a/lib/gitlab/database/background_migration/batched_migration.rb +++ b/lib/gitlab/database/background_migration/batched_migration.rb @@ -68,6 +68,17 @@ module Gitlab ) end + def retry_failed_jobs! + batched_jobs.failed.each_batch(of: 100) do |batch| + self.class.transaction do + batch.lock.each(&:split_and_retry!) + self.active! + end + end + + self.active! + end + def next_min_value last_job&.max_value&.next || min_value end diff --git a/lib/gitlab/database/connection.rb b/lib/gitlab/database/connection.rb index 21861e4fba8..cda6220ee6c 100644 --- a/lib/gitlab/database/connection.rb +++ b/lib/gitlab/database/connection.rb @@ -5,8 +5,6 @@ module Gitlab # Configuration settings and methods for interacting with a PostgreSQL # database, with support for multiple databases. class Connection - DEFAULT_POOL_HEADROOM = 10 - attr_reader :scope # Initializes a new `Database`. @@ -20,20 +18,6 @@ module Gitlab @open_transactions_baseline = 0 end - # We configure the database connection pool size automatically based on - # the configured concurrency. We also add some headroom, to make sure we - # don't run out of connections when more threads besides the 'user-facing' - # ones are running. - # - # Read more about this in - # doc/development/database/client_side_connection_pool.md - def default_pool_size - headroom = - (ENV["DB_POOL_HEADROOM"].presence || DEFAULT_POOL_HEADROOM).to_i - - Gitlab::Runtime.max_threads + headroom - end - def config # The result of this method must not be cached, as other methods may use # it after making configuration changes and expect those changes to be @@ -48,7 +32,7 @@ module Gitlab end def pool_size - config[:pool] || default_pool_size + config[:pool] || Database.default_pool_size end def username @@ -77,7 +61,9 @@ module Gitlab def db_config_with_default_pool_size db_config_object = scope.connection_db_config - config = db_config_object.configuration_hash.merge(pool: default_pool_size) + config = db_config_object + .configuration_hash + .merge(pool: Database.default_pool_size) ActiveRecord::DatabaseConfigurations::HashConfig.new( db_config_object.env_name, @@ -88,7 +74,16 @@ module Gitlab # Disables prepared statements for the current database connection. def disable_prepared_statements - scope.establish_connection(config.merge(prepared_statements: false)) + db_config_object = scope.connection_db_config + config = db_config_object.configuration_hash.merge(prepared_statements: false) + + hash_config = ActiveRecord::DatabaseConfigurations::HashConfig.new( + db_config_object.env_name, + db_config_object.name, + config + ) + + scope.establish_connection(hash_config) end # Check whether the underlying database is in read-only mode @@ -174,8 +169,11 @@ module Gitlab end def exists? - connection - + # We can't _just_ check if `connection` raises an error, as it will + # point to a `ConnectionProxy`, and obtaining those doesn't involve any + # database queries. So instead we obtain the database version, which is + # cached after the first call. + connection.schema_cache.database_version true rescue StandardError false @@ -189,6 +187,19 @@ module Gitlab row['system_identifier'] end + def pg_wal_lsn_diff(location1, location2) + lsn1 = connection.quote(location1) + lsn2 = connection.quote(location2) + + query = <<-SQL.squish + SELECT pg_wal_lsn_diff(#{lsn1}, #{lsn2}) + AS result + SQL + + row = connection.select_all(query).first + row['result'] if row + end + # @param [ActiveRecord::Connection] ar_connection # @return [String] def get_write_location(ar_connection) diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index 08f108eb8e4..bbfbf83222f 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -36,89 +36,42 @@ module Gitlab # Returns a Hash containing the load balancing configuration. def self.configuration - Gitlab::Database.main.config[:load_balancing] || {} - end - - # Returns the maximum replica lag size in bytes. - def self.max_replication_difference - (configuration['max_replication_difference'] || 8.megabytes).to_i - end - - # Returns the maximum lag time for a replica. - def self.max_replication_lag_time - (configuration['max_replication_lag_time'] || 60.0).to_f - end - - # Returns the interval (in seconds) to use for checking the status of a - # replica. - def self.replica_check_interval - (configuration['replica_check_interval'] || 60).to_f - end - - # Returns the additional hosts to use for load balancing. - def self.hosts - configuration['hosts'] || [] - end - - def self.service_discovery_enabled? - configuration.dig('discover', 'record').present? - end - - def self.service_discovery_configuration - conf = configuration['discover'] || {} - - { - nameserver: conf['nameserver'] || 'localhost', - port: conf['port'] || 8600, - record: conf['record'], - record_type: conf['record_type'] || 'A', - interval: conf['interval'] || 60, - disconnect_timeout: conf['disconnect_timeout'] || 120, - use_tcp: conf['use_tcp'] || false - } - end - - def self.pool_size - Gitlab::Database.main.pool_size + @configuration ||= Configuration.for_model(ActiveRecord::Base) end # Returns true if load balancing is to be enabled. def self.enable? return false if Gitlab::Runtime.rake? - return false unless self.configured? - true + configured? end - # Returns true if load balancing has been configured. Since - # Sidekiq does not currently use load balancing, we - # may want Web application servers to detect replication lag by - # posting the write location of the database if load balancing is - # configured. def self.configured? - hosts.any? || service_discovery_enabled? + configuration.load_balancing_enabled? || + configuration.service_discovery_enabled? end def self.start_service_discovery - return unless service_discovery_enabled? + return unless configuration.service_discovery_enabled? - ServiceDiscovery.new(service_discovery_configuration).start + ServiceDiscovery + .new(proxy.load_balancer, **configuration.service_discovery) + .start end # Configures proxying of requests. - def self.configure_proxy(proxy = ConnectionProxy.new(hosts)) - ActiveRecord::Base.load_balancing_proxy = proxy + def self.configure_proxy + lb = LoadBalancer.new(configuration, primary_only: !enable?) + ActiveRecord::Base.load_balancing_proxy = ConnectionProxy.new(lb) # Populate service discovery immediately if it is configured - if service_discovery_enabled? - ServiceDiscovery.new(service_discovery_configuration).perform_service_discovery + if configuration.service_discovery_enabled? + ServiceDiscovery + .new(lb, **configuration.service_discovery) + .perform_service_discovery end end - def self.active_record_models - ActiveRecord::Base.descendants - end - DB_ROLES = [ ROLE_PRIMARY = :primary, ROLE_REPLICA = :replica, @@ -126,24 +79,12 @@ module Gitlab ].freeze # Returns the role (primary/replica) of the database the connection is - # connecting to. At the moment, the connection can only be retrieved by - # Gitlab::Database::LoadBalancer#read or #read_write or from the - # ActiveRecord directly. Therefore, if the load balancer doesn't - # recognize the connection, this method returns the primary role - # directly. In future, we may need to check for other sources. + # connecting to. def self.db_role_for_connection(connection) - return ROLE_UNKNOWN unless connection - - # The connection proxy does not have a role assigned - # as this is dependent on a execution context - return ROLE_UNKNOWN if connection.is_a?(ConnectionProxy) - - # During application init we might receive `NullPool` - return ROLE_UNKNOWN unless connection.respond_to?(:pool) && - connection.pool.respond_to?(:db_config) && - connection.pool.db_config.respond_to?(:name) + db_config = Database.db_config_for_connection(connection) + return ROLE_UNKNOWN unless db_config - if connection.pool.db_config.name.ends_with?(LoadBalancer::REPLICA_SUFFIX) + if db_config.name.ends_with?(LoadBalancer::REPLICA_SUFFIX) ROLE_REPLICA else ROLE_PRIMARY diff --git a/lib/gitlab/database/load_balancing/action_cable_callbacks.rb b/lib/gitlab/database/load_balancing/action_cable_callbacks.rb new file mode 100644 index 00000000000..4feba989a0a --- /dev/null +++ b/lib/gitlab/database/load_balancing/action_cable_callbacks.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module LoadBalancing + module ActionCableCallbacks + def self.install + ::ActionCable::Server::Worker.set_callback :work, :around, &wrapper + ::ActionCable::Channel::Base.set_callback :subscribe, :around, &wrapper + ::ActionCable::Channel::Base.set_callback :unsubscribe, :around, &wrapper + end + + def self.wrapper + lambda do |_, inner| + ::Gitlab::Database::LoadBalancing::Session.current.use_primary! + + inner.call + ensure + ::Gitlab::Database::LoadBalancing.proxy.load_balancer.release_host + ::Gitlab::Database::LoadBalancing::Session.clear_session + end + end + end + end + end +end diff --git a/lib/gitlab/database/load_balancing/configuration.rb b/lib/gitlab/database/load_balancing/configuration.rb new file mode 100644 index 00000000000..238f55fd98e --- /dev/null +++ b/lib/gitlab/database/load_balancing/configuration.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module LoadBalancing + # Configuration settings for a single LoadBalancer instance. + class Configuration + attr_accessor :hosts, :max_replication_difference, + :max_replication_lag_time, :replica_check_interval, + :service_discovery, :model + + # Creates a configuration object for the given ActiveRecord model. + def self.for_model(model) + cfg = model.connection_db_config.configuration_hash.deep_symbolize_keys + lb_cfg = cfg[:load_balancing] || {} + config = new(model) + + if (diff = lb_cfg[:max_replication_difference]) + config.max_replication_difference = diff + end + + if (lag = lb_cfg[:max_replication_lag_time]) + config.max_replication_lag_time = lag.to_f + end + + if (interval = lb_cfg[:replica_check_interval]) + config.replica_check_interval = interval.to_f + end + + if (hosts = lb_cfg[:hosts]) + config.hosts = hosts + end + + discover = lb_cfg[:discover] || {} + + # We iterate over the known/default keys so we don't end up with + # random keys in our configuration hash. + config.service_discovery.each do |key, _| + if (value = discover[key]) + config.service_discovery[key] = value + end + end + + config + end + + def initialize(model, hosts = []) + @max_replication_difference = 8.megabytes + @max_replication_lag_time = 60.0 + @replica_check_interval = 60.0 + @model = model + @hosts = hosts + @service_discovery = { + nameserver: 'localhost', + port: 8600, + record: nil, + record_type: 'A', + interval: 60, + disconnect_timeout: 120, + use_tcp: false + } + end + + def pool_size + # The pool size may change when booting up GitLab, as GitLab enforces + # a certain number of threads. If a Configuration is memoized, this + # can lead to incorrect pool sizes. + # + # To support this scenario, we always attempt to read the pool size + # from the model's configuration. + @model.connection_db_config.configuration_hash[:pool] || + Database.default_pool_size + end + + def load_balancing_enabled? + hosts.any? || service_discovery_enabled? + end + + def service_discovery_enabled? + service_discovery[:record].present? + end + end + end + end +end diff --git a/lib/gitlab/database/load_balancing/connection_proxy.rb b/lib/gitlab/database/load_balancing/connection_proxy.rb index 938f4951532..1be63da8896 100644 --- a/lib/gitlab/database/load_balancing/connection_proxy.rb +++ b/lib/gitlab/database/load_balancing/connection_proxy.rb @@ -34,15 +34,15 @@ module Gitlab ).freeze # hosts - The hosts to use for load balancing. - def initialize(hosts = []) - @load_balancer = LoadBalancer.new(hosts) + def initialize(load_balancer) + @load_balancer = load_balancer end def select_all(arel, name = nil, binds = [], preparable: nil) if arel.respond_to?(:locked) && arel.locked # SELECT ... FOR UPDATE queries should be sent to the primary. - write_using_load_balancer(:select_all, arel, name, binds, - sticky: true) + current_session.write! + write_using_load_balancer(:select_all, arel, name, binds) else read_using_load_balancer(:select_all, arel, name, binds) end @@ -56,7 +56,8 @@ module Gitlab STICKY_WRITES.each do |name| define_method(name) do |*args, **kwargs, &block| - write_using_load_balancer(name, *args, sticky: true, **kwargs, &block) + current_session.write! + write_using_load_balancer(name, *args, **kwargs, &block) end end @@ -65,13 +66,20 @@ module Gitlab track_read_only_transaction! read_using_load_balancer(:transaction, *args, **kwargs, &block) else - write_using_load_balancer(:transaction, *args, sticky: true, **kwargs, &block) + current_session.write! + write_using_load_balancer(:transaction, *args, **kwargs, &block) end ensure untrack_read_only_transaction! end + def respond_to_missing?(name, include_private = false) + @load_balancer.read_write do |connection| + connection.respond_to?(name, include_private) + end + end + # Delegates all unknown messages to a read-write connection. def method_missing(...) if current_session.fallback_to_replicas_for_ambiguous_queries? @@ -102,18 +110,13 @@ module Gitlab # name - The name of the method to call on a connection object. # sticky - If set to true the session will stick to the master after # the write. - def write_using_load_balancer(name, *args, sticky: false, **kwargs, &block) + def write_using_load_balancer(...) if read_only_transaction? raise WriteInsideReadOnlyTransactionError, 'A write query is performed inside a read-only transaction' end @load_balancer.read_write do |connection| - # Sticking has to be enabled before calling the method. Not doing so - # could lead to methods called in a block still being performed on a - # secondary instead of on a primary (when necessary). - current_session.write! if sticky - - connection.send(name, *args, **kwargs, &block) + connection.send(...) end end diff --git a/lib/gitlab/database/load_balancing/host.rb b/lib/gitlab/database/load_balancing/host.rb index 4c5357ae8e3..acd7df0a263 100644 --- a/lib/gitlab/database/load_balancing/host.rb +++ b/lib/gitlab/database/load_balancing/host.rb @@ -29,11 +29,15 @@ module Gitlab @host = host @port = port @load_balancer = load_balancer - @pool = load_balancer.create_replica_connection_pool(::Gitlab::Database::LoadBalancing.pool_size, host, port) + @pool = load_balancer.create_replica_connection_pool( + load_balancer.configuration.pool_size, + host, + port + ) @online = true @last_checked_at = Time.zone.now - interval = ::Gitlab::Database::LoadBalancing.replica_check_interval + interval = load_balancer.configuration.replica_check_interval @intervals = (interval..(interval * 2)).step(0.5).to_a end @@ -108,7 +112,7 @@ module Gitlab def replication_lag_below_threshold? if (lag_time = replication_lag_time) - lag_time <= ::Gitlab::Database::LoadBalancing.max_replication_lag_time + lag_time <= load_balancer.configuration.max_replication_lag_time else false end @@ -125,7 +129,7 @@ module Gitlab # only do this if we haven't replicated in a while so we only need # to connect to the primary when truly necessary. if (lag_size = replication_lag_size) - lag_size <= ::Gitlab::Database::LoadBalancing.max_replication_difference + lag_size <= load_balancer.configuration.max_replication_difference else false end @@ -159,8 +163,6 @@ module Gitlab def primary_write_location load_balancer.primary_write_location - ensure - load_balancer.release_primary_connection end def database_replica_location diff --git a/lib/gitlab/database/load_balancing/host_list.rb b/lib/gitlab/database/load_balancing/host_list.rb index aa731521732..fb3175c7d5d 100644 --- a/lib/gitlab/database/load_balancing/host_list.rb +++ b/lib/gitlab/database/load_balancing/host_list.rb @@ -35,12 +35,6 @@ module Gitlab def hosts=(hosts) @mutex.synchronize do - ::Gitlab::Database::LoadBalancing::Logger.info( - event: :host_list_update, - message: "Updating the host list for service discovery", - host_list_length: hosts.length, - old_host_list_length: @hosts.length - ) @hosts = hosts unsafe_shuffle end diff --git a/lib/gitlab/database/load_balancing/load_balancer.rb b/lib/gitlab/database/load_balancing/load_balancer.rb index e3f5d0ac470..9b00b323301 100644 --- a/lib/gitlab/database/load_balancing/load_balancer.rb +++ b/lib/gitlab/database/load_balancing/load_balancer.rb @@ -12,12 +12,22 @@ module Gitlab REPLICA_SUFFIX = '_replica' - attr_reader :host_list - - # hosts - The hostnames/addresses of the additional databases. - def initialize(hosts = [], model = ActiveRecord::Base) - @model = model - @host_list = HostList.new(hosts.map { |addr| Host.new(addr, self) }) + attr_reader :host_list, :configuration + + # configuration - An instance of `LoadBalancing::Configuration` that + # contains the configuration details (such as the hosts) + # for this load balancer. + # primary_only - If set, the replicas are ignored and the primary is + # always used. + def initialize(configuration, primary_only: false) + @configuration = configuration + @primary_only = primary_only + @host_list = + if primary_only + HostList.new([PrimaryHost.new(self)]) + else + HostList.new(configuration.hosts.map { |addr| Host.new(addr, self) }) + end end def disconnect!(timeout: 120) @@ -169,7 +179,11 @@ module Gitlab when ActiveRecord::StatementInvalid, ActionView::Template::Error # After connecting to the DB Rails will wrap query errors using this # class. - connection_error?(error.cause) + if (cause = error.cause) + connection_error?(cause) + else + false + end when *CONNECTION_ERRORS true else @@ -213,26 +227,26 @@ module Gitlab .establish_connection(replica_db_config) end - private - # ActiveRecord::ConnectionAdapters::ConnectionHandler handles fetching, # and caching for connections pools for each "connection", so we # leverage that. def pool ActiveRecord::Base.connection_handler.retrieve_connection_pool( - @model.connection_specification_name, + @configuration.model.connection_specification_name, role: ActiveRecord::Base.writing_role, shard: ActiveRecord::Base.default_shard ) end + private + def ensure_caching! host.enable_query_cache! unless host.query_cache_enabled end def request_cache - base = RequestStore[:gitlab_load_balancer] ||= {} - base[pool] ||= {} + base = SafeRequestStore[:gitlab_load_balancer] ||= {} + base[self] ||= {} end end end diff --git a/lib/gitlab/database/load_balancing/primary_host.rb b/lib/gitlab/database/load_balancing/primary_host.rb new file mode 100644 index 00000000000..e379652c260 --- /dev/null +++ b/lib/gitlab/database/load_balancing/primary_host.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module LoadBalancing + # A host that wraps the primary database connection. + # + # This class is used to always enable load balancing as if replicas exist, + # without the need for extra database connections. This ensures that code + # using the load balancer doesn't have to handle the case where load + # balancing is enabled, but no replicas have been configured (= the + # default case). + class PrimaryHost + def initialize(load_balancer) + @load_balancer = load_balancer + end + + def release_connection + # no-op as releasing primary connections isn't needed. + nil + end + + def enable_query_cache! + # This could mess up the primary connection, so we make this a no-op + nil + end + + def disable_query_cache! + # This could mess up the primary connection, so we make this a no-op + nil + end + + def query_cache_enabled + @load_balancer.pool.query_cache_enabled + end + + def connection + @load_balancer.pool.connection + end + + def disconnect!(timeout: 120) + nil + end + + def offline! + nil + end + + def online? + true + end + + def primary_write_location + @load_balancer.primary_write_location + end + + def database_replica_location + row = query_and_release(<<-SQL.squish) + SELECT pg_last_wal_replay_lsn()::text AS location + SQL + + row['location'] if row.any? + rescue *Host::CONNECTION_ERRORS + nil + end + + def caught_up?(_location) + true + end + + def query_and_release(sql) + connection.select_all(sql).first || {} + rescue StandardError + {} + ensure + release_connection + end + end + end + end +end diff --git a/lib/gitlab/database/load_balancing/service_discovery.rb b/lib/gitlab/database/load_balancing/service_discovery.rb index 251961c8246..dfd4892371c 100644 --- a/lib/gitlab/database/load_balancing/service_discovery.rb +++ b/lib/gitlab/database/load_balancing/service_discovery.rb @@ -13,11 +13,17 @@ module Gitlab # balancer with said hosts. Requests may continue to use the old hosts # until they complete. class ServiceDiscovery + EmptyDnsResponse = Class.new(StandardError) + attr_reader :interval, :record, :record_type, :disconnect_timeout, :load_balancer MAX_SLEEP_ADJUSTMENT = 10 + MAX_DISCOVERY_RETRIES = 3 + + RETRY_DELAY_RANGE = (0.1..0.2).freeze + RECORD_TYPES = { 'A' => Net::DNS::A, 'SRV' => Net::DNS::SRV @@ -43,14 +49,14 @@ module Gitlab # use_tcp - Use TCP instaed of UDP to look up resources # load_balancer - The load balancer instance to use def initialize( + load_balancer, nameserver:, port:, record:, record_type: 'A', interval: 60, disconnect_timeout: 120, - use_tcp: false, - load_balancer: LoadBalancing.proxy.load_balancer + use_tcp: false ) @nameserver = nameserver @port = port @@ -76,15 +82,23 @@ module Gitlab end def perform_service_discovery - refresh_if_necessary - rescue StandardError => error - # Any exceptions that might occur should be reported to - # Sentry, instead of silently terminating this thread. - Gitlab::ErrorTracking.track_exception(error) - - Gitlab::AppLogger.error( - "Service discovery encountered an error: #{error.message}" - ) + MAX_DISCOVERY_RETRIES.times do + return refresh_if_necessary + rescue StandardError => error + # Any exceptions that might occur should be reported to + # Sentry, instead of silently terminating this thread. + Gitlab::ErrorTracking.track_exception(error) + + Gitlab::Database::LoadBalancing::Logger.error( + event: :service_discovery_failure, + message: "Service discovery encountered an error: #{error.message}", + host_list_length: load_balancer.host_list.length + ) + + # Slightly randomize the retry delay so that, in the case of a total + # dns outage, all starting services do not pressure the dns server at the same time. + sleep(rand(RETRY_DELAY_RANGE)) + end interval end @@ -99,7 +113,22 @@ module Gitlab current = addresses_from_load_balancer - replace_hosts(from_dns) if from_dns != current + if from_dns != current + ::Gitlab::Database::LoadBalancing::Logger.info( + event: :host_list_update, + message: "Updating the host list for service discovery", + host_list_length: from_dns.length, + old_host_list_length: current.length + ) + replace_hosts(from_dns) + else + ::Gitlab::Database::LoadBalancing::Logger.info( + event: :host_list_unchanged, + message: "Unchanged host list for service discovery", + host_list_length: from_dns.length, + old_host_list_length: current.length + ) + end interval end @@ -141,6 +170,8 @@ module Gitlab addresses_from_srv_record(response) end + raise EmptyDnsResponse if addresses.empty? + # Addresses are sorted so we can directly compare the old and new # addresses, without having to use any additional data structures. [new_wait_time_for(resources), addresses.sort] diff --git a/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb b/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb index 0e36ebbc3ee..518a812b406 100644 --- a/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb +++ b/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb @@ -4,13 +4,15 @@ module Gitlab module Database module LoadBalancing class SidekiqClientMiddleware + include Gitlab::Utils::StrongMemoize + def call(worker_class, job, _queue, _redis_pool) # Mailers can't be constantized worker_class = worker_class.to_s.safe_constantize if load_balancing_enabled?(worker_class) job['worker_data_consistency'] = worker_class.get_data_consistency - set_data_consistency_location!(job) unless location_already_provided?(job) + set_data_consistency_locations!(job) unless job['wal_locations'] else job['worker_data_consistency'] = ::WorkerAttributes::DEFAULT_DATA_CONSISTENCY end @@ -27,16 +29,23 @@ module Gitlab worker_class.get_data_consistency_feature_flag_enabled? end - def set_data_consistency_location!(job) - if Session.current.use_primary? - job['database_write_location'] = load_balancer.primary_write_location - else - job['database_replica_location'] = load_balancer.host.database_replica_location - end + def set_data_consistency_locations!(job) + # Once we add support for multiple databases to our load balancer, we would use something like this: + # job['wal_locations'] = Gitlab::Database::DATABASES.transform_values do |connection| + # connection.load_balancer.primary_write_location + # end + # + job['wal_locations'] = { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => wal_location } if wal_location end - def location_already_provided?(job) - job['database_replica_location'] || job['database_write_location'] + def wal_location + strong_memoize(:wal_location) do + if Session.current.use_primary? + load_balancer.primary_write_location + else + load_balancer.host.database_replica_location + end + end end def load_balancer diff --git a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb index 0551750568a..15f8f0fb240 100644 --- a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb +++ b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb @@ -29,7 +29,7 @@ module Gitlab private def clear - load_balancer.release_host + release_hosts Session.clear_session end @@ -40,10 +40,11 @@ module Gitlab def select_load_balancing_strategy(worker_class, job) return :primary unless load_balancing_available?(worker_class) - location = job['database_write_location'] || job['database_replica_location'] - return :primary_no_wal unless location + wal_locations = get_wal_locations(job) + + return :primary_no_wal unless wal_locations - if replica_caught_up?(location) + if all_databases_has_replica_caught_up?(wal_locations) # Happy case: we can read from a replica. retried_before?(worker_class, job) ? :replica_retried : :replica elsif can_retry?(worker_class, job) @@ -55,6 +56,19 @@ module Gitlab end end + def get_wal_locations(job) + job['dedup_wal_locations'] || job['wal_locations'] || legacy_wal_location(job) + end + + # Already scheduled jobs could still contain legacy database write location. + # TODO: remove this in the next iteration + # https://gitlab.com/gitlab-org/gitlab/-/issues/338213 + def legacy_wal_location(job) + wal_location = job['database_write_location'] || job['database_replica_location'] + + { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => wal_location } if wal_location + end + def load_balancing_available?(worker_class) worker_class.include?(::ApplicationWorker) && worker_class.utilizes_load_balancing_capabilities? && @@ -75,12 +89,26 @@ module Gitlab job['retry_count'].nil? end - def load_balancer - LoadBalancing.proxy.load_balancer + def all_databases_has_replica_caught_up?(wal_locations) + wal_locations.all? do |_config_name, location| + # Once we add support for multiple databases to our load balancer, we would use something like this: + # Gitlab::Database::DATABASES[config_name].load_balancer.select_up_to_date_host(location) + load_balancer.select_up_to_date_host(location) + end end - def replica_caught_up?(location) - load_balancer.select_up_to_date_host(location) + def release_hosts + # Once we add support for multiple databases to our load balancer, we would use something like this: + # connection.load_balancer.primary_write_location + # + # Gitlab::Database::DATABASES.values.each do |connection| + # connection.load_balancer.release_host + # end + load_balancer.release_host + end + + def load_balancer + LoadBalancing.proxy.load_balancer end end end diff --git a/lib/gitlab/database/migration.rb b/lib/gitlab/database/migration.rb new file mode 100644 index 00000000000..b2248b0f4eb --- /dev/null +++ b/lib/gitlab/database/migration.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class Migration + module LockRetriesConcern + extend ActiveSupport::Concern + + class_methods do + def enable_lock_retries! + @enable_lock_retries = true # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def enable_lock_retries? + @enable_lock_retries + end + end + + delegate :enable_lock_retries?, to: :class + end + + # This implements a simple versioning scheme for migration helpers. + # + # We need to be able to version helpers, so we can change their behavior without + # altering the behavior of already existing migrations in incompatible ways. + # + # We can continue to change the behavior of helpers without bumping the version here, + # *if* the change is backwards-compatible. + # + # If not, we would typically override the helper method in a new MigrationHelpers::V[0-9]+ + # class and create a new entry with a bumped version below. + # + # We use major version bumps to indicate significant changes and minor version bumps + # to indicate backwards-compatible or otherwise minor changes (e.g. a Rails version bump). + # However, this hasn't been strictly formalized yet. + MIGRATION_CLASSES = { + 1.0 => Class.new(ActiveRecord::Migration[6.1]) do + include LockRetriesConcern + include Gitlab::Database::MigrationHelpers::V2 + end + }.freeze + + def self.[](version) + MIGRATION_CLASSES[version] || raise(ArgumentError, "Unknown migration version: #{version}") + end + + # The current version to be used in new migrations + def self.current_version + 1.0 + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 23d9b16dc09..9968096b1f6 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -73,6 +73,7 @@ module Gitlab end end + # @deprecated Use `create_table` in V2 instead # # Creates a new table, optionally allowing the caller to add check constraints to the table. # Aside from that addition, this method should behave identically to Rails' `create_table` method. @@ -380,6 +381,8 @@ module Gitlab # The timings can be controlled via the +timing_configuration+ parameter. # If the lock was not acquired within the retry period, a last attempt is made without using +lock_timeout+. # + # Note this helper uses subtransactions when run inside an already open transaction. + # # ==== Examples # # Invoking without parameters # with_lock_retries do @@ -411,7 +414,8 @@ module Gitlab raise_on_exhaustion = !!kwargs.delete(:raise_on_exhaustion) merged_args = { klass: self.class, - logger: Gitlab::BackgroundMigration::Logger + logger: Gitlab::BackgroundMigration::Logger, + allow_savepoints: true }.merge(kwargs) Gitlab::Database::WithLockRetries.new(**merged_args) @@ -600,17 +604,17 @@ module Gitlab # new_column - The name of the new column. # trigger_name - The name of the trigger to use (optional). def install_rename_triggers(table, old, new, trigger_name: nil) - Gitlab::Database::UnidirectionalCopyTrigger.on_table(table).create(old, new, trigger_name: trigger_name) + Gitlab::Database::UnidirectionalCopyTrigger.on_table(table, connection: connection).create(old, new, trigger_name: trigger_name) end # Removes the triggers used for renaming a column concurrently. def remove_rename_triggers(table, trigger) - Gitlab::Database::UnidirectionalCopyTrigger.on_table(table).drop(trigger) + Gitlab::Database::UnidirectionalCopyTrigger.on_table(table, connection: connection).drop(trigger) end # Returns the (base) name to use for triggers when renaming columns. def rename_trigger_name(table, old, new) - Gitlab::Database::UnidirectionalCopyTrigger.on_table(table).name(old, new) + Gitlab::Database::UnidirectionalCopyTrigger.on_table(table, connection: connection).name(old, new) end # Changes the type of a column concurrently. @@ -968,42 +972,7 @@ module Gitlab # columns - The name, or array of names, of the column(s) that we want to convert to bigint. # primary_key - The name of the primary key column (most often :id) def initialize_conversion_of_integer_to_bigint(table, columns, primary_key: :id) - unless table_exists?(table) - raise "Table #{table} does not exist" - end - - unless column_exists?(table, primary_key) - raise "Column #{primary_key} does not exist on #{table}" - end - - columns = Array.wrap(columns) - columns.each do |column| - next if column_exists?(table, column) - - raise ArgumentError, "Column #{column} does not exist on #{table}" - end - - check_trigger_permissions!(table) - - conversions = columns.to_h { |column| [column, convert_to_bigint_column(column)] } - - with_lock_retries do - conversions.each do |(source_column, temporary_name)| - column = column_for(table, source_column) - - if (column.name.to_s == primary_key.to_s) || !column.null - # If the column to be converted is either a PK or is defined as NOT NULL, - # set it to `NOT NULL DEFAULT 0` and we'll copy paste the correct values bellow - # That way, we skip the expensive validation step required to add - # a NOT NULL constraint at the end of the process - add_column(table, temporary_name, :bigint, default: column.default || 0, null: false) - else - add_column(table, temporary_name, :bigint, default: column.default) - end - end - - install_rename_triggers(table, conversions.keys, conversions.values) - end + create_temporary_columns_and_triggers(table, columns, primary_key: primary_key, data_type: :bigint) end # Reverts `initialize_conversion_of_integer_to_bigint` @@ -1019,6 +988,17 @@ module Gitlab temporary_columns.each { |column| remove_column(table, column) } end + alias_method :cleanup_conversion_of_integer_to_bigint, :revert_initialize_conversion_of_integer_to_bigint + + # Reverts `cleanup_conversion_of_integer_to_bigint` + # + # table - The name of the database table containing the columns + # columns - The name, or array of names, of the column(s) that we have converted to bigint. + # primary_key - The name of the primary key column (most often :id) + + def restore_conversion_of_integer_to_bigint(table, columns, primary_key: :id) + create_temporary_columns_and_triggers(table, columns, primary_key: primary_key, data_type: :int) + end # Backfills the new columns used in an integer-to-bigint conversion using background migrations. # @@ -1400,13 +1380,11 @@ into similar problems in the future (e.g. when new tables are created). # validate - Whether to validate the constraint in this call # def add_check_constraint(table, check, constraint_name, validate: true) - validate_check_constraint_name!(constraint_name) - # Transactions would result in ALTER TABLE locks being held for the # duration of the transaction, defeating the purpose of this method. - if transaction_open? - raise 'add_check_constraint can not be run inside a transaction' - end + validate_not_in_transaction!(:add_check_constraint) + + validate_check_constraint_name!(constraint_name) if check_constraint_exists?(table, constraint_name) warning_message = <<~MESSAGE @@ -1451,6 +1429,10 @@ into similar problems in the future (e.g. when new tables are created). end def remove_check_constraint(table, constraint_name) + # This is technically not necessary, but aligned with add_check_constraint + # and allows us to continue use with_lock_retries here + validate_not_in_transaction!(:remove_check_constraint) + validate_check_constraint_name!(constraint_name) # DROP CONSTRAINT requires an EXCLUSIVE lock @@ -1649,6 +1631,45 @@ into similar problems in the future (e.g. when new tables are created). private + def create_temporary_columns_and_triggers(table, columns, primary_key: :id, data_type: :bigint) + unless table_exists?(table) + raise "Table #{table} does not exist" + end + + unless column_exists?(table, primary_key) + raise "Column #{primary_key} does not exist on #{table}" + end + + columns = Array.wrap(columns) + columns.each do |column| + next if column_exists?(table, column) + + raise ArgumentError, "Column #{column} does not exist on #{table}" + end + + check_trigger_permissions!(table) + + conversions = columns.to_h { |column| [column, convert_to_bigint_column(column)] } + + with_lock_retries do + conversions.each do |(source_column, temporary_name)| + column = column_for(table, source_column) + + if (column.name.to_s == primary_key.to_s) || !column.null + # If the column to be converted is either a PK or is defined as NOT NULL, + # set it to `NOT NULL DEFAULT 0` and we'll copy paste the correct values bellow + # That way, we skip the expensive validation step required to add + # a NOT NULL constraint at the end of the process + add_column(table, temporary_name, data_type, default: column.default || 0, null: false) + else + add_column(table, temporary_name, data_type, default: column.default) + end + end + + install_rename_triggers(table, conversions.keys, conversions.values) + end + end + def validate_check_constraint_name!(constraint_name) if constraint_name.to_s.length > MAX_IDENTIFIER_NAME_LENGTH raise "The maximum allowed constraint name is #{MAX_IDENTIFIER_NAME_LENGTH} characters" diff --git a/lib/gitlab/database/migration_helpers/cascading_namespace_settings.rb b/lib/gitlab/database/migration_helpers/cascading_namespace_settings.rb index eecf96acb30..d9ef5ab462e 100644 --- a/lib/gitlab/database/migration_helpers/cascading_namespace_settings.rb +++ b/lib/gitlab/database/migration_helpers/cascading_namespace_settings.rb @@ -31,10 +31,8 @@ module Gitlab namespace_options = options.merge(null: true, default: nil) - with_lock_retries do - add_column(:namespace_settings, setting_name, type, namespace_options) - add_column(:namespace_settings, lock_column_name, :boolean, default: false, null: false) - end + add_column(:namespace_settings, setting_name, type, namespace_options) + add_column(:namespace_settings, lock_column_name, :boolean, default: false, null: false) add_column(:application_settings, setting_name, type, options) add_column(:application_settings, lock_column_name, :boolean, default: false, null: false) @@ -43,10 +41,8 @@ module Gitlab def remove_cascading_namespace_setting(setting_name) lock_column_name = "lock_#{setting_name}".to_sym - with_lock_retries do - remove_column(:namespace_settings, setting_name) if column_exists?(:namespace_settings, setting_name) - remove_column(:namespace_settings, lock_column_name) if column_exists?(:namespace_settings, lock_column_name) - end + remove_column(:namespace_settings, setting_name) if column_exists?(:namespace_settings, setting_name) + remove_column(:namespace_settings, lock_column_name) if column_exists?(:namespace_settings, lock_column_name) remove_column(:application_settings, setting_name) if column_exists?(:application_settings, setting_name) remove_column(:application_settings, lock_column_name) if column_exists?(:application_settings, lock_column_name) diff --git a/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb b/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb new file mode 100644 index 00000000000..30601bffd7a --- /dev/null +++ b/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module MigrationHelpers + module LooseForeignKeyHelpers + include Gitlab::Database::SchemaHelpers + + DELETED_RECORDS_INSERT_FUNCTION_NAME = 'insert_into_loose_foreign_keys_deleted_records' + + def track_record_deletions(table) + execute(<<~SQL) + CREATE TRIGGER #{record_deletion_trigger_name(table)} + AFTER DELETE ON #{table} REFERENCING OLD TABLE AS old_table + FOR EACH STATEMENT + EXECUTE FUNCTION #{DELETED_RECORDS_INSERT_FUNCTION_NAME}(); + SQL + end + + def untrack_record_deletions(table) + drop_trigger(table, record_deletion_trigger_name(table)) + end + + private + + def record_deletion_trigger_name(table) + "#{table}_loose_fk_trigger" + end + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers/v2.rb b/lib/gitlab/database/migration_helpers/v2.rb index f20a9b30fa7..0e7f6075196 100644 --- a/lib/gitlab/database/migration_helpers/v2.rb +++ b/lib/gitlab/database/migration_helpers/v2.rb @@ -6,6 +6,118 @@ module Gitlab module V2 include Gitlab::Database::MigrationHelpers + # Superseded by `create_table` override below + def create_table_with_constraints(*_) + raise <<~EOM + #create_table_with_constraints is not supported anymore - use #create_table instead, for example: + + create_table :db_guides do |t| + t.bigint :stars, default: 0, null: false + t.text :title, limit: 128 + t.text :notes, limit: 1024 + + t.check_constraint 'stars > 1000', name: 'so_many_stars' + end + + See https://docs.gitlab.com/ee/development/database/strings_and_the_text_data_type.html + EOM + end + + # Creates a new table, optionally allowing the caller to add text limit constraints to the table. + # This method only extends Rails' `create_table` method + # + # Example: + # + # create_table :db_guides do |t| + # t.bigint :stars, default: 0, null: false + # t.text :title, limit: 128 + # t.text :notes, limit: 1024 + # + # t.check_constraint 'stars > 1000', name: 'so_many_stars' + # end + # + # See Rails' `create_table` for more info on the available arguments. + # + # When adding foreign keys to other tables, consider wrapping the call into a with_lock_retries block + # to avoid traffic stalls. + def create_table(table_name, *args, **kwargs, &block) + helper_context = self + + super do |t| + t.define_singleton_method(:text) do |column_name, **kwargs| + limit = kwargs.delete(:limit) + + super(column_name, **kwargs) + + if limit + # rubocop:disable GitlabSecurity/PublicSend + name = helper_context.send(:text_limit_name, table_name, column_name) + # rubocop:enable GitlabSecurity/PublicSend + + column_name = helper_context.quote_column_name(column_name) + definition = "char_length(#{column_name}) <= #{limit}" + + t.check_constraint(definition, name: name) + end + end + + t.instance_eval(&block) unless block.nil? + end + end + + # Executes the block with a retry mechanism that alters the +lock_timeout+ and +sleep_time+ between attempts. + # The timings can be controlled via the +timing_configuration+ parameter. + # If the lock was not acquired within the retry period, a last attempt is made without using +lock_timeout+. + # + # In order to retry the block, the method wraps the block into a transaction. + # + # When called inside an open transaction it will execute the block directly if lock retries are enabled + # with `enable_lock_retries!` at migration level, otherwise it will raise an error. + # + # ==== Examples + # # Invoking without parameters + # with_lock_retries do + # drop_table :my_table + # end + # + # # Invoking with custom +timing_configuration+ + # t = [ + # [1.second, 1.second], + # [2.seconds, 2.seconds] + # ] + # + # with_lock_retries(timing_configuration: t) do + # drop_table :my_table # this will be retried twice + # end + # + # # Disabling the retries using an environment variable + # > export DISABLE_LOCK_RETRIES=true + # + # with_lock_retries do + # drop_table :my_table # one invocation, it will not retry at all + # end + # + # ==== Parameters + # * +timing_configuration+ - [[ActiveSupport::Duration, ActiveSupport::Duration], ...] lock timeout for the block, sleep time before the next iteration, defaults to `Gitlab::Database::WithLockRetries::DEFAULT_TIMING_CONFIGURATION` + # * +logger+ - [Gitlab::JsonLogger] + # * +env+ - [Hash] custom environment hash, see the example with `DISABLE_LOCK_RETRIES` + def with_lock_retries(*args, **kwargs, &block) + if transaction_open? + if enable_lock_retries? + Gitlab::AppLogger.warn 'Lock retries already enabled, executing the block directly' + yield + else + raise <<~EOF + #{__callee__} can not be run inside an already open transaction + + Use migration-level lock retries instead, see https://docs.gitlab.com/ee/development/migration_style_guide.html#retry-mechanism-when-acquiring-database-locks + EOF + end + else + super(*args, **kwargs.merge(allow_savepoints: false), &block) + end + end + # Renames a column without requiring downtime. # # Concurrent renames work by using database triggers to ensure both the diff --git a/lib/gitlab/database/migrations/lock_retry_mixin.rb b/lib/gitlab/database/migrations/lock_retry_mixin.rb new file mode 100644 index 00000000000..fff0f35e33c --- /dev/null +++ b/lib/gitlab/database/migrations/lock_retry_mixin.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module LockRetryMixin + module ActiveRecordMigrationProxyLockRetries + def migration_class + migration.class + end + + def enable_lock_retries? + # regular AR migrations don't have this, + # only ones inheriting from Gitlab::Database::Migration have + return false unless migration.respond_to?(:enable_lock_retries?) + + migration.enable_lock_retries? + end + end + + module ActiveRecordMigratorLockRetries + # We patch the original method to start a transaction + # using the WithLockRetries methodology for the whole migration. + def ddl_transaction(migration, &block) + if use_transaction?(migration) && migration.enable_lock_retries? + Gitlab::Database::WithLockRetries.new( + klass: migration.migration_class, + logger: Gitlab::BackgroundMigration::Logger + ).run(raise_on_exhaustion: false, &block) + else + super + end + end + end + + def self.patch! + ActiveRecord::MigrationProxy.prepend(ActiveRecordMigrationProxyLockRetries) + ActiveRecord::Migrator.prepend(ActiveRecordMigratorLockRetries) + end + end + end + end +end diff --git a/lib/gitlab/database/partitioning.rb b/lib/gitlab/database/partitioning.rb new file mode 100644 index 00000000000..bbde2063c41 --- /dev/null +++ b/lib/gitlab/database/partitioning.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + def self.register_models(models) + registered_models.merge(models) + end + + def self.registered_models + @registered_models ||= Set.new + end + + def self.sync_partitions(models_to_sync = registered_models) + MultiDatabasePartitionManager.new(models_to_sync).sync_partitions + end + end + end +end diff --git a/lib/gitlab/database/partitioning/monthly_strategy.rb b/lib/gitlab/database/partitioning/monthly_strategy.rb index 7992c2fdaa7..4cdde5bf2f1 100644 --- a/lib/gitlab/database/partitioning/monthly_strategy.rb +++ b/lib/gitlab/database/partitioning/monthly_strategy.rb @@ -4,17 +4,18 @@ module Gitlab module Database module Partitioning class MonthlyStrategy - attr_reader :model, :partitioning_key, :retain_for + attr_reader :model, :partitioning_key, :retain_for, :retain_non_empty_partitions # We create this many partitions in the future HEADROOM = 6.months delegate :table_name, to: :model - def initialize(model, partitioning_key, retain_for: nil) + def initialize(model, partitioning_key, retain_for: nil, retain_non_empty_partitions: false) @model = model @partitioning_key = partitioning_key @retain_for = retain_for + @retain_non_empty_partitions = retain_non_empty_partitions end def current_partitions @@ -29,7 +30,10 @@ module Gitlab end def extra_partitions - current_partitions - desired_partitions + partitions = current_partitions - desired_partitions + partitions.reject!(&:holds_data?) if retain_non_empty_partitions + + partitions end private diff --git a/lib/gitlab/database/partitioning/multi_database_partition_manager.rb b/lib/gitlab/database/partitioning/multi_database_partition_manager.rb new file mode 100644 index 00000000000..5a93e3fb1fb --- /dev/null +++ b/lib/gitlab/database/partitioning/multi_database_partition_manager.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + class MultiDatabasePartitionManager + def initialize(models) + @models = models + end + + def sync_partitions + Gitlab::AppLogger.info(message: "Syncing dynamic postgres partitions") + + models.each do |model| + Gitlab::Database::SharedModel.using_connection(model.connection) do + Gitlab::AppLogger.debug(message: "Switched database connection", + connection_name: connection_name, + table_name: model.table_name) + + PartitionManager.new(model).sync_partitions + end + end + + Gitlab::AppLogger.info(message: "Finished sync of dynamic postgres partitions") + end + + private + + attr_reader :models + + def connection_name + Gitlab::Database::SharedModel.connection.pool.db_config.name + end + end + end + end +end diff --git a/lib/gitlab/database/partitioning/partition_manager.rb b/lib/gitlab/database/partitioning/partition_manager.rb index 7e433ecdd39..8742c0ff166 100644 --- a/lib/gitlab/database/partitioning/partition_manager.rb +++ b/lib/gitlab/database/partitioning/partition_manager.rb @@ -6,60 +6,49 @@ module Gitlab class PartitionManager UnsafeToDetachPartitionError = Class.new(StandardError) - def self.register(model) - raise ArgumentError, "Only models with a #partitioning_strategy can be registered." unless model.respond_to?(:partitioning_strategy) - - models << model - end - - def self.models - @models ||= Set.new - end - LEASE_TIMEOUT = 1.minute MANAGEMENT_LEASE_KEY = 'database_partition_management_%s' RETAIN_DETACHED_PARTITIONS_FOR = 1.week - attr_reader :models - - def initialize(models = self.class.models) - @models = models + def initialize(model) + @model = model end def sync_partitions - Gitlab::AppLogger.info("Checking state of dynamic postgres partitions") + Gitlab::AppLogger.info(message: "Checking state of dynamic postgres partitions", table_name: model.table_name) - models.each do |model| - # Double-checking before getting the lease: - # The prevailing situation is no missing partitions and no extra partitions - next if missing_partitions(model).empty? && extra_partitions(model).empty? + # Double-checking before getting the lease: + # The prevailing situation is no missing partitions and no extra partitions + return if missing_partitions.empty? && extra_partitions.empty? - only_with_exclusive_lease(model, lease_key: MANAGEMENT_LEASE_KEY) do - partitions_to_create = missing_partitions(model) - create(partitions_to_create) unless partitions_to_create.empty? + only_with_exclusive_lease(model, lease_key: MANAGEMENT_LEASE_KEY) do + partitions_to_create = missing_partitions + create(partitions_to_create) unless partitions_to_create.empty? - if Feature.enabled?(:partition_pruning, default_enabled: :yaml) - partitions_to_detach = extra_partitions(model) - detach(partitions_to_detach) unless partitions_to_detach.empty? - end + if Feature.enabled?(:partition_pruning, default_enabled: :yaml) + partitions_to_detach = extra_partitions + detach(partitions_to_detach) unless partitions_to_detach.empty? end - rescue StandardError => e - Gitlab::AppLogger.error(message: "Failed to create / detach partition(s)", - table_name: model.table_name, - exception_class: e.class, - exception_message: e.message) end + rescue StandardError => e + Gitlab::AppLogger.error(message: "Failed to create / detach partition(s)", + table_name: model.table_name, + exception_class: e.class, + exception_message: e.message) end private - def missing_partitions(model) + attr_reader :model + delegate :connection, to: :model + + def missing_partitions return [] unless connection.table_exists?(model.table_name) model.partitioning_strategy.missing_partitions end - def extra_partitions(model) + def extra_partitions return [] unless connection.table_exists?(model.table_name) model.partitioning_strategy.extra_partitions @@ -74,8 +63,9 @@ module Gitlab end def create(partitions) - connection.transaction do - with_lock_retries do + # with_lock_retries starts a requires_new transaction most of the time, but not on the last iteration + with_lock_retries do + connection.transaction(requires_new: false) do # so we open a transaction here if not already in progress partitions.each do |partition| connection.execute partition.to_sql @@ -88,8 +78,9 @@ module Gitlab end def detach(partitions) - connection.transaction do - with_lock_retries do + # with_lock_retries starts a requires_new transaction most of the time, but not on the last iteration + with_lock_retries do + connection.transaction(requires_new: false) do # so we open a transaction here if not already in progress partitions.each { |p| detach_one_partition(p) } end end @@ -119,13 +110,10 @@ module Gitlab def with_lock_retries(&block) Gitlab::Database::WithLockRetries.new( klass: self.class, - logger: Gitlab::AppLogger + logger: Gitlab::AppLogger, + connection: connection ).run(&block) end - - def connection - ActiveRecord::Base.connection - end end end end diff --git a/lib/gitlab/database/partitioning/partition_monitoring.rb b/lib/gitlab/database/partitioning/partition_monitoring.rb index 6963ecd2cc1..e5b561fc447 100644 --- a/lib/gitlab/database/partitioning/partition_monitoring.rb +++ b/lib/gitlab/database/partitioning/partition_monitoring.rb @@ -6,7 +6,7 @@ module Gitlab class PartitionMonitoring attr_reader :models - def initialize(models = PartitionManager.models) + def initialize(models = Gitlab::Database::Partitioning.registered_models) @models = models end diff --git a/lib/gitlab/database/partitioning/time_partition.rb b/lib/gitlab/database/partitioning/time_partition.rb index 1221f042530..e09ca483549 100644 --- a/lib/gitlab/database/partitioning/time_partition.rb +++ b/lib/gitlab/database/partitioning/time_partition.rb @@ -69,6 +69,10 @@ module Gitlab partition_name <=> other.partition_name end + def holds_data? + conn.execute("SELECT 1 FROM #{fully_qualified_partition} LIMIT 1").ntuples > 0 + end + private def date_or_nil(obj) diff --git a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb index f1aa7871245..bd8ed677d77 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb @@ -6,6 +6,8 @@ module Gitlab module ForeignKeyHelpers include ::Gitlab::Database::SchemaHelpers + ERROR_SCOPE = 'foreign keys' + # Adds a foreign key with only minimal locking on the tables involved. # # In concept it works similarly to add_concurrent_foreign_key, but we have @@ -32,6 +34,8 @@ module Gitlab # name - The name of the foreign key. # def add_concurrent_partitioned_foreign_key(source, target, column:, on_delete: :cascade, name: nil) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + partition_options = { column: column, on_delete: on_delete, diff --git a/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb index c0cc97de276..c9a3b5caf79 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/index_helpers.rb @@ -7,6 +7,8 @@ module Gitlab include Gitlab::Database::MigrationHelpers include Gitlab::Database::SchemaHelpers + ERROR_SCOPE = 'index' + # Concurrently creates a new index on a partitioned table. In concept this works similarly to # `add_concurrent_index`, and won't block reads or writes on the table while the index is being built. # @@ -21,6 +23,8 @@ module Gitlab # # See Rails' `add_index` for more info on the available arguments. def add_concurrent_partitioned_index(table_name, column_names, options = {}) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + raise ArgumentError, 'A name is required for indexes added to partitioned tables' unless options[:name] partitioned_table = find_partitioned_table(table_name) @@ -57,6 +61,8 @@ module Gitlab # # remove_concurrent_partitioned_index_by_name :users, 'index_name_goes_here' def remove_concurrent_partitioned_index_by_name(table_name, index_name) + assert_not_in_transaction_block(scope: ERROR_SCOPE) + find_partitioned_table(table_name) unless index_name_exists?(table_name, index_name) diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index 9ccbdc9930e..0dc9f92e4c8 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -431,7 +431,7 @@ module Gitlab replace_table = Gitlab::Database::Partitioning::ReplaceTable.new(original_table_name.to_s, replacement_table_name, replaced_table_name, primary_key_name) - with_lock_retries do + transaction do drop_sync_trigger(original_table_name) replace_table.perform do |sql| diff --git a/lib/gitlab/database/postgres_foreign_key.rb b/lib/gitlab/database/postgres_foreign_key.rb index 94f74724295..72640f8785d 100644 --- a/lib/gitlab/database/postgres_foreign_key.rb +++ b/lib/gitlab/database/postgres_foreign_key.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresForeignKey < ApplicationRecord + class PostgresForeignKey < SharedModel self.primary_key = :oid scope :by_referenced_table_identifier, ->(identifier) do diff --git a/lib/gitlab/database/postgres_partition.rb b/lib/gitlab/database/postgres_partition.rb index 7da60d8375d..eb080904f73 100644 --- a/lib/gitlab/database/postgres_partition.rb +++ b/lib/gitlab/database/postgres_partition.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresPartition < ActiveRecord::Base + class PostgresPartition < SharedModel self.primary_key = :identifier belongs_to :postgres_partitioned_table, foreign_key: 'parent_identifier', primary_key: 'identifier' diff --git a/lib/gitlab/database/postgres_partitioned_table.rb b/lib/gitlab/database/postgres_partitioned_table.rb index 5d2eaa22ee4..3bd342f940f 100644 --- a/lib/gitlab/database/postgres_partitioned_table.rb +++ b/lib/gitlab/database/postgres_partitioned_table.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresPartitionedTable < ActiveRecord::Base + class PostgresPartitionedTable < SharedModel DYNAMIC_PARTITION_STRATEGIES = %w[range list].freeze self.primary_key = :identifier diff --git a/lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin.rb b/lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin.rb index a2e7f4befab..59ca06b5aca 100644 --- a/lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin.rb +++ b/lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin.rb @@ -7,7 +7,7 @@ module Gitlab extend ActiveSupport::Concern def dump_schema_information # :nodoc: - Gitlab::Database::SchemaMigrations.touch_all(self) + Gitlab::Database::SchemaMigrations.touch_all(self) if Gitlab.dev_or_test_env? nil end diff --git a/lib/gitlab/database/rename_table_helpers.rb b/lib/gitlab/database/rename_table_helpers.rb index 7f5af038c6d..e881c0e5455 100644 --- a/lib/gitlab/database/rename_table_helpers.rb +++ b/lib/gitlab/database/rename_table_helpers.rb @@ -4,27 +4,27 @@ module Gitlab module Database module RenameTableHelpers def rename_table_safely(old_table_name, new_table_name) - with_lock_retries do + transaction do rename_table(old_table_name, new_table_name) execute("CREATE VIEW #{old_table_name} AS SELECT * FROM #{new_table_name}") end end def undo_rename_table_safely(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("DROP VIEW IF EXISTS #{old_table_name}") rename_table(new_table_name, old_table_name) end end def finalize_table_rename(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("DROP VIEW IF EXISTS #{old_table_name}") end end def undo_finalize_table_rename(old_table_name, new_table_name) - with_lock_retries do + transaction do execute("CREATE VIEW #{old_table_name} AS SELECT * FROM #{new_table_name}") end end diff --git a/lib/gitlab/database/schema_migrations/context.rb b/lib/gitlab/database/schema_migrations/context.rb index 35105121bbd..a95f85c6bef 100644 --- a/lib/gitlab/database/schema_migrations/context.rb +++ b/lib/gitlab/database/schema_migrations/context.rb @@ -6,7 +6,7 @@ module Gitlab class Context attr_reader :connection - DEFAULT_SCHEMA_MIGRATIONS_PATH = "db/schema_migrations" + class_attribute :default_schema_migrations_path, default: 'db/schema_migrations' def initialize(connection) @connection = connection @@ -30,7 +30,7 @@ module Gitlab end def database_schema_migrations_path - @connection.pool.db_config.configuration_hash[:schema_migrations_path] || DEFAULT_SCHEMA_MIGRATIONS_PATH + @connection.pool.db_config.configuration_hash[:schema_migrations_path] || self.class.default_schema_migrations_path end end end diff --git a/lib/gitlab/database/shared_model.rb b/lib/gitlab/database/shared_model.rb new file mode 100644 index 00000000000..8f256758961 --- /dev/null +++ b/lib/gitlab/database/shared_model.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class SharedModel < ActiveRecord::Base + self.abstract_class = true + + class << self + def using_connection(connection) + raise 'cannot nest connection overrides for shared models' unless overriding_connection.nil? + + self.overriding_connection = connection + + yield + ensure + self.overriding_connection = nil + end + + def connection + if connection = self.overriding_connection + connection + else + super + end + end + + private + + def overriding_connection + Thread.current[:overriding_connection] + end + + def overriding_connection=(connection) + Thread.current[:overriding_connection] = connection + end + end + end + end +end diff --git a/lib/gitlab/database/transaction/context.rb b/lib/gitlab/database/transaction/context.rb index a50dd30b75b..a902537f02e 100644 --- a/lib/gitlab/database/transaction/context.rb +++ b/lib/gitlab/database/transaction/context.rb @@ -6,9 +6,8 @@ module Gitlab class Context attr_reader :context - LOG_DEPTH_THRESHOLD = 8 - LOG_SAVEPOINTS_THRESHOLD = 32 - LOG_DURATION_S_THRESHOLD = 300 + LOG_SAVEPOINTS_THRESHOLD = 1 # 1 `SAVEPOINT` created in a transaction + LOG_DURATION_S_THRESHOLD = 120 # transaction that is running for 2 minutes or longer LOG_THROTTLE_DURATION = 1 def initialize @@ -19,6 +18,10 @@ module Gitlab @context[:start_time] = current_timestamp end + def set_depth(depth) + @context[:depth] = [@context[:depth].to_i, depth].max + end + def increment_savepoints @context[:savepoints] = @context[:savepoints].to_i + 1 end @@ -31,42 +34,33 @@ module Gitlab @context[:releases] = @context[:releases].to_i + 1 end - def set_depth(depth) - @context[:depth] = [@context[:depth].to_i, depth].max - end - def track_sql(sql) (@context[:queries] ||= []).push(sql) end + def track_backtrace(backtrace) + cleaned_backtrace = Gitlab::BacktraceCleaner.clean_backtrace(backtrace) + (@context[:backtraces] ||= []).push(cleaned_backtrace) + end + def duration return unless @context[:start_time].present? current_timestamp - @context[:start_time] end - def depth_threshold_exceeded? - @context[:depth].to_i > LOG_DEPTH_THRESHOLD - end - def savepoints_threshold_exceeded? - @context[:savepoints].to_i > LOG_SAVEPOINTS_THRESHOLD + @context[:savepoints].to_i >= LOG_SAVEPOINTS_THRESHOLD end def duration_threshold_exceeded? - duration.to_i > LOG_DURATION_S_THRESHOLD - end - - def log_savepoints? - depth_threshold_exceeded? || savepoints_threshold_exceeded? - end - - def log_duration? - duration_threshold_exceeded? + duration.to_i >= LOG_DURATION_S_THRESHOLD end def should_log? - !logged_already? && (log_savepoints? || log_duration?) + return false if logged_already? + + savepoints_threshold_exceeded? || duration_threshold_exceeded? end def commit @@ -77,6 +71,10 @@ module Gitlab log(:rollback) end + def backtraces + @context[:backtraces].to_a + end + private def queries @@ -110,7 +108,8 @@ module Gitlab savepoints_count: @context[:savepoints].to_i, rollbacks_count: @context[:rollbacks].to_i, releases_count: @context[:releases].to_i, - sql: queries + sql: queries, + savepoint_backtraces: backtraces } application_info(attributes) diff --git a/lib/gitlab/database/transaction/observer.rb b/lib/gitlab/database/transaction/observer.rb index 7888f0916e3..ad6886a3d52 100644 --- a/lib/gitlab/database/transaction/observer.rb +++ b/lib/gitlab/database/transaction/observer.rb @@ -21,9 +21,10 @@ module Gitlab context.set_start_time context.set_depth(0) context.track_sql(event.payload[:sql]) - elsif cmd.start_with?('SAVEPOINT ') + elsif cmd.start_with?('SAVEPOINT', 'EXCEPTION') context.set_depth(manager.open_transactions) context.increment_savepoints + context.track_backtrace(caller) elsif cmd.start_with?('ROLLBACK TO SAVEPOINT') context.increment_rollbacks elsif cmd.start_with?('RELEASE SAVEPOINT ') diff --git a/lib/gitlab/database/with_lock_retries.rb b/lib/gitlab/database/with_lock_retries.rb index bbf8f133f0f..f9d467ae5cc 100644 --- a/lib/gitlab/database/with_lock_retries.rb +++ b/lib/gitlab/database/with_lock_retries.rb @@ -61,13 +61,15 @@ module Gitlab [10.seconds, 10.minutes] ].freeze - def initialize(logger: NULL_LOGGER, timing_configuration: DEFAULT_TIMING_CONFIGURATION, klass: nil, env: ENV) + def initialize(logger: NULL_LOGGER, allow_savepoints: true, timing_configuration: DEFAULT_TIMING_CONFIGURATION, klass: nil, env: ENV, connection: ActiveRecord::Base.connection) @logger = logger @klass = klass + @allow_savepoints = allow_savepoints @timing_configuration = timing_configuration @env = env @current_iteration = 1 @log_params = { method: 'with_lock_retries', class: klass.to_s } + @connection = connection end # Executes a block of code, retrying it whenever a database lock can't be acquired in time @@ -95,7 +97,7 @@ module Gitlab run_block_with_lock_timeout rescue ActiveRecord::LockWaitTimeout if retry_with_lock_timeout? - disable_idle_in_transaction_timeout if ActiveRecord::Base.connection.transaction_open? + disable_idle_in_transaction_timeout if connection.transaction_open? wait_until_next_retry reset_db_settings @@ -115,14 +117,16 @@ module Gitlab private - attr_reader :logger, :env, :block, :current_iteration, :log_params, :timing_configuration + attr_reader :logger, :env, :block, :current_iteration, :log_params, :timing_configuration, :connection def run_block block.call end def run_block_with_lock_timeout - ActiveRecord::Base.transaction(requires_new: true) do + raise "WithLockRetries should not run inside already open transaction" if connection.transaction_open? && @allow_savepoints.blank? + + connection.transaction(requires_new: true) do # rubocop:disable Performance/ActiveRecordSubtransactions execute("SET LOCAL lock_timeout TO '#{current_lock_timeout_in_ms}ms'") log(message: 'Lock timeout is set', current_iteration: current_iteration, lock_timeout_in_ms: current_lock_timeout_in_ms) @@ -149,7 +153,7 @@ module Gitlab log(message: "Couldn't acquire lock to perform the migration", current_iteration: current_iteration) log(message: "Executing the migration without lock timeout", current_iteration: current_iteration) - disable_lock_timeout if ActiveRecord::Base.connection.transaction_open? + disable_lock_timeout if connection.transaction_open? run_block @@ -165,7 +169,7 @@ module Gitlab end def execute(statement) - ActiveRecord::Base.connection.execute(statement) + connection.execute(statement) end def retry_count |