diff options
8 files changed, 117 insertions, 4 deletions
diff --git a/changelogs/unreleased/197957-projects-api-improve-response-time-for-created_at-desc-order-and-v.yml b/changelogs/unreleased/197957-projects-api-improve-response-time-for-created_at-desc-order-and-v.yml new file mode 100644 index 00000000000..018d26c59df --- /dev/null +++ b/changelogs/unreleased/197957-projects-api-improve-response-time-for-created_at-desc-order-and-v.yml @@ -0,0 +1,5 @@ +--- +title: Improve API response for descending internal project searches +merge_request: 28038 +author: +type: performance diff --git a/db/migrate/20200215222507_drop_forked_project_links_fk.rb b/db/migrate/20200215222507_drop_forked_project_links_fk.rb index f3ee36e9037..0be7a57ed0e 100644 --- a/db/migrate/20200215222507_drop_forked_project_links_fk.rb +++ b/db/migrate/20200215222507_drop_forked_project_links_fk.rb @@ -8,17 +8,21 @@ class DropForkedProjectLinksFk < ActiveRecord::Migration[6.0] disable_ddl_transaction! def up + # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction with_lock_retries do remove_foreign_key_if_exists :forked_project_links, column: :forked_to_project_id end + # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction end def down unless foreign_key_exists?(:forked_project_links, :projects, column: :forked_to_project_id) + # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction with_lock_retries do # rubocop: disable Migration/AddConcurrentForeignKey add_foreign_key :forked_project_links, :projects, column: :forked_to_project_id, on_delete: :cascade, validate: false end + # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction end fk_name = concurrent_foreign_key_name(:forked_project_links, :forked_to_project_id, prefix: 'fk_rails_') diff --git a/db/migrate/20200304085423_add_user_type.rb b/db/migrate/20200304085423_add_user_type.rb index 68db44c6847..355a16897f4 100644 --- a/db/migrate/20200304085423_add_user_type.rb +++ b/db/migrate/20200304085423_add_user_type.rb @@ -5,8 +5,6 @@ class AddUserType < ActiveRecord::Migration[6.0] DOWNTIME = false - disable_ddl_transaction! - def up with_lock_retries do add_column :users, :user_type, :integer, limit: 2 diff --git a/db/migrate/20200325183636_add_api_index_for_internal_projects.rb b/db/migrate/20200325183636_add_api_index_for_internal_projects.rb new file mode 100644 index 00000000000..ce47f1f3ded --- /dev/null +++ b/db/migrate/20200325183636_add_api_index_for_internal_projects.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddApiIndexForInternalProjects < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + INTERNAL_PROJECTS_INDEX_NAME = "index_projects_api_created_at_id_for_vis10" + + disable_ddl_transaction! + + def up + add_concurrent_index :projects, [:created_at, :id], + where: "visibility_level = 10 AND pending_delete = false", + name: INTERNAL_PROJECTS_INDEX_NAME + end + + def down + remove_concurrent_index_by_name :projects, INTERNAL_PROJECTS_INDEX_NAME + end +end diff --git a/db/structure.sql b/db/structure.sql index a972ef7085a..7c6c883196c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9685,6 +9685,8 @@ CREATE INDEX index_projects_api_created_at_id_for_archived ON public.projects US CREATE INDEX index_projects_api_created_at_id_for_archived_vis20 ON public.projects USING btree (created_at, id) WHERE ((archived = true) AND (visibility_level = 20) AND (pending_delete = false)); +CREATE INDEX index_projects_api_created_at_id_for_vis10 ON public.projects USING btree (created_at, id) WHERE ((visibility_level = 10) AND (pending_delete = false)); + CREATE INDEX index_projects_api_last_activity_at_id_desc ON public.projects USING btree (last_activity_at, id DESC); CREATE INDEX index_projects_api_name_id_desc ON public.projects USING btree (name, id DESC); @@ -12839,5 +12841,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200323134519 20200324115359 20200325160952 +20200325183636 \. diff --git a/doc/user/packages/container_registry/index.md b/doc/user/packages/container_registry/index.md index c511dbb5d01..37072eea39e 100644 --- a/doc/user/packages/container_registry/index.md +++ b/doc/user/packages/container_registry/index.md @@ -487,13 +487,13 @@ Examples: - Select all tags, keep at least 1 tag per image, expire any tag older than 14 days, run once a month, and the policy is enabled: ```shell - curl --request PUT --header 'Content-Type: application/json;charset=UTF-8' --header "PRIVATE-TOKEN: <your_access_token>" --data-binary '{"container_expiration_policy_attributes":{"cadence":"1month","enabled":true,"keep_n":1,"older_than":"14d","name_regex":".*"}' 'https://gitlab.example.com/api/v4/projects/2' + curl --request PUT --header 'Content-Type: application/json;charset=UTF-8' --header "PRIVATE-TOKEN: <your_access_token>" --data-binary '{"container_expiration_policy_attributes":{"cadence":"1month","enabled":true,"keep_n":1,"older_than":"14d","name_regex":".*"}}' 'https://gitlab.example.com/api/v4/projects/2' ``` - Select only tags with a name that contains `stable`, keep at least 50 tag per image, expire any tag older than 7 days, run every day, and the policy is enabled: ```shell - curl --request PUT --header 'Content-Type: application/json;charset=UTF-8' --header "PRIVATE-TOKEN: <your_access_token>" --data-binary '{"container_expiration_policy_attributes":{"cadence":"1day","enabled":true,"keep_n":50"older_than":"7d","name_regex":"*stable"}' 'https://gitlab.example.com/api/v4/projects/2' + curl --request PUT --header 'Content-Type: application/json;charset=UTF-8' --header "PRIVATE-TOKEN: <your_access_token>" --data-binary '{"container_expiration_policy_attributes":{"cadence":"1day","enabled":true,"keep_n":50"older_than":"7d","name_regex":"*stable"}}' 'https://gitlab.example.com/api/v4/projects/2' ``` See the API documentation for further details: [Edit project](../../../api/projects.md#edit-project). diff --git a/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb b/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb new file mode 100644 index 00000000000..ebd91dd5a6e --- /dev/null +++ b/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that prevents usage of `with_lock_retries` with `disable_ddl_transaction!` + class WithLockRetriesWithoutDdlTransaction < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`with_lock_retries` cannot be used with disabled DDL transactions (`disable_ddl_transaction!`). ' \ + 'Please remove the `disable_ddl_transaction!` call from your migration.'.freeze + + def_node_matcher :disable_ddl_transaction?, <<~PATTERN + (send _ :disable_ddl_transaction!) + PATTERN + + def_node_matcher :with_lock_retries?, <<~PATTERN + (send _ :with_lock_retries) + PATTERN + + def on_send(node) + return unless in_migration?(node) + return unless with_lock_retries?(node) + + node.each_ancestor(:begin) do |begin_node| + disable_ddl_transaction_node = begin_node.children.find { |n| disable_ddl_transaction?(n) } + + add_offense(node, location: :expression) if disable_ddl_transaction_node + end + end + end + end + end +end diff --git a/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb new file mode 100644 index 00000000000..b42a4a14c67 --- /dev/null +++ b/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/with_lock_retries_without_ddl_transaction' + +describe RuboCop::Cop::Migration::WithLockRetriesWithoutDdlTransaction do + include CopHelper + + let(:valid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; def up; with_lock_retries {}; end; end' } + let(:invalid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; disable_ddl_transaction!; def up; with_lock_retries {}; end; end' } + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when `with_lock_retries` is used with `disable_ddl_transaction!` method' do + inspect_source(invalid_source) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when `with_lock_retries` is used inside an `up` method' do + inspect_source(valid_source) + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(invalid_source) + + expect(cop.offenses.size).to eq(0) + end + end +end |