diff options
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/users.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb | 41 | ||||
-rw-r--r-- | lib/gitlab/database/migration_helpers.rb | 88 | ||||
-rw-r--r-- | lib/gitlab/database/read_only_relation.rb | 16 | ||||
-rw-r--r-- | lib/gitlab/group_hierarchy.rb | 15 |
5 files changed, 158 insertions, 4 deletions
diff --git a/lib/api/users.rb b/lib/api/users.rb index 1825c90a23b..bdebda58d3f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -88,7 +88,7 @@ module API user = User.find_by(id: params[:id]) not_found!('User') unless user && can?(current_user, :read_user, user) - opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : {} + opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : { with: Entities::User } present user, opts end diff --git a/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb b/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb new file mode 100644 index 00000000000..b1411be3016 --- /dev/null +++ b/lib/gitlab/background_migration/delete_conflicting_redirect_routes_range.rb @@ -0,0 +1,41 @@ +module Gitlab + module BackgroundMigration + class DeleteConflictingRedirectRoutesRange + class Route < ActiveRecord::Base + self.table_name = 'routes' + end + + class RedirectRoute < ActiveRecord::Base + self.table_name = 'redirect_routes' + end + + # start_id - The start ID of the range of events to process + # end_id - The end ID of the range to process. + def perform(start_id, end_id) + return unless migrate? + + conflicts = RedirectRoute.where(routes_match_redirects_clause(start_id, end_id)) + num_rows = conflicts.delete_all + + Rails.logger.info("Gitlab::BackgroundMigration::DeleteConflictingRedirectRoutesRange [#{start_id}, #{end_id}] - Deleted #{num_rows} redirect routes that were conflicting with routes.") + end + + def migrate? + Route.table_exists? && RedirectRoute.table_exists? + end + + def routes_match_redirects_clause(start_id, end_id) + <<~ROUTES_MATCH_REDIRECTS + EXISTS ( + SELECT 1 FROM routes + WHERE ( + LOWER(redirect_routes.path) = LOWER(routes.path) + OR LOWER(redirect_routes.path) LIKE LOWER(CONCAT(routes.path, '/%')) + ) + AND routes.id BETWEEN #{start_id} AND #{end_id} + ) + ROUTES_MATCH_REDIRECTS + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index fb14798efe6..2c35da8f1aa 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -1,6 +1,9 @@ module Gitlab module Database module MigrationHelpers + BACKGROUND_MIGRATION_BATCH_SIZE = 1000 # Number of rows to process per job + BACKGROUND_MIGRATION_JOB_BUFFER_SIZE = 1000 # Number of jobs to bulk queue at a time + # Adds `created_at` and `updated_at` columns with timezone information. # # This method is an improved version of Rails' built-in method `add_timestamps`. @@ -653,6 +656,91 @@ into similar problems in the future (e.g. when new tables are created). EOF end end + + # Bulk queues background migration jobs for an entire table, batched by ID range. + # "Bulk" meaning many jobs will be pushed at a time for efficiency. + # If you need a delay interval per job, then use `queue_background_migration_jobs_by_range_at_intervals`. + # + # model_class - The table being iterated over + # job_class_name - The background migration job class as a string + # batch_size - The maximum number of rows per job + # + # Example: + # + # class Route < ActiveRecord::Base + # include EachBatch + # self.table_name = 'routes' + # end + # + # bulk_queue_background_migration_jobs_by_range(Route, 'ProcessRoutes') + # + # Where the model_class includes EachBatch, and the background migration exists: + # + # class Gitlab::BackgroundMigration::ProcessRoutes + # def perform(start_id, end_id) + # # do something + # end + # end + def bulk_queue_background_migration_jobs_by_range(model_class, job_class_name, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) + raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') + + jobs = [] + + model_class.each_batch(of: batch_size) do |relation| + start_id, end_id = relation.pluck('MIN(id), MAX(id)').first + + if jobs.length >= BACKGROUND_MIGRATION_JOB_BUFFER_SIZE + # Note: This code path generally only helps with many millions of rows + # We push multiple jobs at a time to reduce the time spent in + # Sidekiq/Redis operations. We're using this buffer based approach so we + # don't need to run additional queries for every range. + BackgroundMigrationWorker.perform_bulk(jobs) + jobs.clear + end + + jobs << [job_class_name, [start_id, end_id]] + end + + BackgroundMigrationWorker.perform_bulk(jobs) unless jobs.empty? + end + + # Queues background migration jobs for an entire table, batched by ID range. + # Each job is scheduled with a `delay_interval` in between. + # If you use a small interval, then some jobs may run at the same time. + # + # model_class - The table being iterated over + # job_class_name - The background migration job class as a string + # delay_interval - The duration between each job's scheduled time (must respond to `to_f`) + # batch_size - The maximum number of rows per job + # + # Example: + # + # class Route < ActiveRecord::Base + # include EachBatch + # self.table_name = 'routes' + # end + # + # queue_background_migration_jobs_by_range_at_intervals(Route, 'ProcessRoutes', 1.minute) + # + # Where the model_class includes EachBatch, and the background migration exists: + # + # class Gitlab::BackgroundMigration::ProcessRoutes + # def perform(start_id, end_id) + # # do something + # end + # end + def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) + raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') + + model_class.each_batch(of: batch_size) do |relation, index| + start_id, end_id = relation.pluck('MIN(id), MAX(id)').first + + # `BackgroundMigrationWorker.bulk_perform_in` schedules all jobs for + # the same time, which is not helpful in most cases where we wish to + # spread the work over time. + BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id]) + end + end end end end diff --git a/lib/gitlab/database/read_only_relation.rb b/lib/gitlab/database/read_only_relation.rb new file mode 100644 index 00000000000..4571ad122ce --- /dev/null +++ b/lib/gitlab/database/read_only_relation.rb @@ -0,0 +1,16 @@ +module Gitlab + module Database + # Module that can be injected into a ActiveRecord::Relation to make it + # read-only. + module ReadOnlyRelation + [:delete, :delete_all, :update, :update_all].each do |method| + define_method(method) do |*args| + raise( + ActiveRecord::ReadOnlyRecord, + "This relation is marked as read-only" + ) + end + end + end + end +end diff --git a/lib/gitlab/group_hierarchy.rb b/lib/gitlab/group_hierarchy.rb index 5a31e56cb30..635f52131f9 100644 --- a/lib/gitlab/group_hierarchy.rb +++ b/lib/gitlab/group_hierarchy.rb @@ -22,7 +22,7 @@ module Gitlab def base_and_ancestors return ancestors_base unless Group.supports_nested_groups? - base_and_ancestors_cte.apply_to(model.all) + read_only(base_and_ancestors_cte.apply_to(model.all)) end # Returns a relation that includes the descendants_base set of groups @@ -30,7 +30,7 @@ module Gitlab def base_and_descendants return descendants_base unless Group.supports_nested_groups? - base_and_descendants_cte.apply_to(model.all) + read_only(base_and_descendants_cte.apply_to(model.all)) end # Returns a relation that includes the base groups, their ancestors, @@ -67,11 +67,13 @@ module Gitlab union = SQL::Union.new([model.unscoped.from(ancestors_table), model.unscoped.from(descendants_table)]) - model + relation = model .unscoped .with .recursive(ancestors.to_arel, descendants.to_arel) .from("(#{union.to_sql}) #{model.table_name}") + + read_only(relation) end private @@ -107,5 +109,12 @@ module Gitlab def groups_table model.arel_table end + + def read_only(relation) + # relations using a CTE are not safe to use with update_all as it will + # throw away the CTE, hence we mark them as read-only. + relation.extend(Gitlab::Database::ReadOnlyRelation) + relation + end end end |