From c172bb9967f280e05bd904188d60a959dff10f00 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 9 Sep 2020 18:08:48 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../unreleased/tc-external-mr-diff-ssf.yml | 5 +++ ...42811_create_merge_request_diff_details.rb | 30 ++++++++++++++++ db/schema_migrations/20200827142811 | 1 + db/structure.sql | 29 ++++++++++++++++ doc/.vale/gitlab/AlertBoxStyle.yml | 6 ++++ .../monitoring/prometheus/gitlab_metrics.md | 6 ++++ doc/api/README.md | 34 +++++++++---------- doc/api/geo_nodes.md | 6 ++++ doc/development/geo/framework.md | 2 +- doc/user/gitlab_com/index.md | 9 +++++ .../import_export/project/import_export.yml | 12 +++++++ spec/bin/feature_flag_spec.rb | 20 +++++------ spec/factories/merge_request_diffs.rb | 10 +++++- spec/factories/merge_requests.rb | 10 ++++++ spec/lib/gitlab/import_export/all_models.yml | 3 ++ 15 files changed, 151 insertions(+), 32 deletions(-) create mode 100644 changelogs/unreleased/tc-external-mr-diff-ssf.yml create mode 100644 db/migrate/20200827142811_create_merge_request_diff_details.rb create mode 100644 db/schema_migrations/20200827142811 diff --git a/changelogs/unreleased/tc-external-mr-diff-ssf.yml b/changelogs/unreleased/tc-external-mr-diff-ssf.yml new file mode 100644 index 00000000000..bea1e3b42bf --- /dev/null +++ b/changelogs/unreleased/tc-external-mr-diff-ssf.yml @@ -0,0 +1,5 @@ +--- +title: 'Geo: Add migrations for registry and details tables for external MR diff replication' +merge_request: 34248 +author: +type: added diff --git a/db/migrate/20200827142811_create_merge_request_diff_details.rb b/db/migrate/20200827142811_create_merge_request_diff_details.rb new file mode 100644 index 00000000000..29b070bc675 --- /dev/null +++ b/db/migrate/20200827142811_create_merge_request_diff_details.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class CreateMergeRequestDiffDetails < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + unless table_exists?(:merge_request_diff_details) + with_lock_retries do + create_table :merge_request_diff_details, id: false do |t| + t.references :merge_request_diff, primary_key: true, null: false, foreign_key: { on_delete: :cascade } + t.datetime_with_timezone :verification_retry_at + t.datetime_with_timezone :verified_at + t.integer :verification_retry_count, limit: 2 + t.binary :verification_checksum, using: 'verification_checksum::bytea' + t.text :verification_failure + end + end + end + + add_text_limit :merge_request_diff_details, :verification_failure, 255 + end + + def down + drop_table :merge_request_diff_details + end +end diff --git a/db/schema_migrations/20200827142811 b/db/schema_migrations/20200827142811 new file mode 100644 index 00000000000..c1edecb9575 --- /dev/null +++ b/db/schema_migrations/20200827142811 @@ -0,0 +1 @@ +0e2b3433577946177876f14ec414a1653c1edeaa796eea24f12740958f964442 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index faed2300ccc..340375a655f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13146,6 +13146,25 @@ CREATE TABLE public.merge_request_diff_commits ( message text ); +CREATE TABLE public.merge_request_diff_details ( + merge_request_diff_id bigint NOT NULL, + verification_retry_at timestamp with time zone, + verified_at timestamp with time zone, + verification_retry_count smallint, + verification_checksum bytea, + verification_failure text, + CONSTRAINT check_81429e3622 CHECK ((char_length(verification_failure) <= 255)) +); + +CREATE SEQUENCE public.merge_request_diff_details_merge_request_diff_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE public.merge_request_diff_details_merge_request_diff_id_seq OWNED BY public.merge_request_diff_details.merge_request_diff_id; + CREATE TABLE public.merge_request_diff_files ( merge_request_diff_id integer NOT NULL, relative_order integer NOT NULL, @@ -17248,6 +17267,8 @@ ALTER TABLE ONLY public.merge_request_blocks ALTER COLUMN id SET DEFAULT nextval ALTER TABLE ONLY public.merge_request_context_commits ALTER COLUMN id SET DEFAULT nextval('public.merge_request_context_commits_id_seq'::regclass); +ALTER TABLE ONLY public.merge_request_diff_details ALTER COLUMN merge_request_diff_id SET DEFAULT nextval('public.merge_request_diff_details_merge_request_diff_id_seq'::regclass); + ALTER TABLE ONLY public.merge_request_diffs ALTER COLUMN id SET DEFAULT nextval('public.merge_request_diffs_id_seq'::regclass); ALTER TABLE ONLY public.merge_request_metrics ALTER COLUMN id SET DEFAULT nextval('public.merge_request_metrics_id_seq'::regclass); @@ -18377,6 +18398,9 @@ ALTER TABLE ONLY public.merge_request_blocks ALTER TABLE ONLY public.merge_request_context_commits ADD CONSTRAINT merge_request_context_commits_pkey PRIMARY KEY (id); +ALTER TABLE ONLY public.merge_request_diff_details + ADD CONSTRAINT merge_request_diff_details_pkey PRIMARY KEY (merge_request_diff_id); + ALTER TABLE ONLY public.merge_request_diffs ADD CONSTRAINT merge_request_diffs_pkey PRIMARY KEY (id); @@ -20204,6 +20228,8 @@ CREATE UNIQUE INDEX index_merge_request_diff_commits_on_mr_diff_id_and_order ON CREATE INDEX index_merge_request_diff_commits_on_sha ON public.merge_request_diff_commits USING btree (sha); +CREATE INDEX index_merge_request_diff_details_on_merge_request_diff_id ON public.merge_request_diff_details USING btree (merge_request_diff_id); + CREATE UNIQUE INDEX index_merge_request_diff_files_on_mr_diff_id_and_order ON public.merge_request_diff_files USING btree (merge_request_diff_id, relative_order); CREATE INDEX index_merge_request_diffs_by_id_partial ON public.merge_request_diffs USING btree (id) WHERE ((files_count > 0) AND ((NOT stored_externally) OR (stored_externally IS NULL))); @@ -22871,6 +22897,9 @@ ALTER TABLE ONLY public.deployment_merge_requests ALTER TABLE ONLY public.analytics_language_trend_repository_languages ADD CONSTRAINT fk_rails_86cc9aef5f FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY public.merge_request_diff_details + ADD CONSTRAINT fk_rails_86f4d24ecd FOREIGN KEY (merge_request_diff_id) REFERENCES public.merge_request_diffs(id) ON DELETE CASCADE; + ALTER TABLE ONLY public.clusters_applications_crossplane ADD CONSTRAINT fk_rails_87186702df FOREIGN KEY (cluster_id) REFERENCES public.clusters(id) ON DELETE CASCADE; diff --git a/doc/.vale/gitlab/AlertBoxStyle.yml b/doc/.vale/gitlab/AlertBoxStyle.yml index 8f9e444edc1..06743d95ea9 100644 --- a/doc/.vale/gitlab/AlertBoxStyle.yml +++ b/doc/.vale/gitlab/AlertBoxStyle.yml @@ -3,6 +3,12 @@ # # Makes sure alert boxes follow standard formatting. # +# Checks for 4 known issues: +# - Alert boxes with no colon, or colon outside the bold text +# - Known incorrect capitalization of the most commonly used alert box text +# - Alert boxes with the note text on the same line +# - Alert boxes using blockquote formatting, like "> **Note:**" +# # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence message: 'Alert box "%s" must use the formatting in the style guide.' diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index f8d62f411ce..c747f86ab1a 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -186,6 +186,12 @@ configuration option in `gitlab.yml`. These metrics are served from the | `geo_terraform_states_registry` | Gauge | 13.3 | Number of terraform states in the registry | `url` | | `global_search_bulk_cron_queue_size` | Gauge | 12.10 | Number of database records waiting to be synchronized to Elasticsearch | | | `global_search_awaiting_indexing_queue_size` | Gauge | 13.2 | Number of database updates waiting to be synchronized to Elasticsearch while indexing is paused | | +| `geo_merge_request_diffs` | Gauge | 13.4 | Number of merge request diffs on primary | `url` | +| `geo_merge_request_diffs_checksummed` | Gauge | 13.4 | Number of merge request diffs checksummed on primary | `url` | +| `geo_merge_request_diffs_checksum_failed` | Gauge | 13.4 | Number of merge request diffs failed to calculate the checksum on primary | `url` | +| `geo_merge_request_diffs_synced` | Gauge | 13.4 | Number of syncable merge request diffs synced on secondary | `url` | +| `geo_merge_request_diffs_failed` | Gauge | 13.4 | Number of syncable merge request diffs failed to sync on secondary | `url` | +| `geo_merge_request_diffs_registry` | Gauge | 13.4 | Number of merge request diffs in the registry | `url` | ## Database load balancing metrics **(PREMIUM ONLY)** diff --git a/doc/api/README.md b/doc/api/README.md index 600ca9bcde3..33b8acda5bb 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -342,9 +342,12 @@ curl --request PUT --header "PRIVATE-TOKEN: " "https://gitlab #### Pagination `Link` header -[`Link` headers](https://www.w3.org/wiki/LinkHeader) are sent back with each -response. They have `rel` set to `prev`/`next`/`first`/`last` and contain the relevant -URL. Please use these links instead of generating your own URLs. +[`Link` headers](https://www.w3.org/wiki/LinkHeader) are returned with each +response. They have `rel` set to `prev`/`next`/`first`/`last` and contain the +relevant URL. Be sure to use these links instead of generating your own URLs. + +NOTE: **Note:** +For GitLab.com users, [some pagination headers may not be returned](../user/gitlab_com/index.md#pagination-response-headers). In the cURL example below, we limit the output to 3 items per page (`per_page=3`) and we request the second page (`page=2`) of [comments](notes.md) of the issue @@ -377,24 +380,19 @@ X-Total-Pages: 3 #### Other pagination headers -Additional pagination headers are also sent back. +GitLab also returns the following additional pagination headers: -| Header | Description | -| ------ | ----------- | -| `X-Total` | The total number of items | -| `X-Total-Pages` | The total number of pages | -| `X-Per-Page` | The number of items per page | +| Header | Description | +| --------------- | --------------------------------------------- | +| `X-Total` | The total number of items | +| `X-Total-Pages` | The total number of pages | +| `X-Per-Page` | The number of items per page | | `X-Page` | The index of the current page (starting at 1) | -| `X-Next-Page` | The index of the next page | -| `X-Prev-Page` | The index of the previous page | +| `X-Next-Page` | The index of the next page | +| `X-Prev-Page` | The index of the previous page | -CAUTION: **Caution:** -For performance reasons since -[GitLab 11.8](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/23931) -and **behind the `api_kaminari_count_with_limit` -[feature flag](../development/feature_flags/index.md)**, if the number of resources is -more than 10,000, the `X-Total` and `X-Total-Pages` headers as well as the -`rel="last"` `Link` are not present in the response headers. +NOTE: **Note:** +For GitLab.com users, [some pagination headers may not be returned](../user/gitlab_com/index.md#pagination-response-headers). ### Keyset-based pagination diff --git a/doc/api/geo_nodes.md b/doc/api/geo_nodes.md index e52bcbcdaa0..82b7ef79a4a 100644 --- a/doc/api/geo_nodes.md +++ b/doc/api/geo_nodes.md @@ -442,6 +442,12 @@ Example response: "last_successful_status_check_timestamp": 1510125024, "version": "10.3.0", "revision": "33d33a096a", + "merge_request_diffs_count": 12, + "merge_request_diffs_checksummed_count": 8, + "merge_request_diffs_checksum_failed_count": 0, + "merge_request_diffs_registry_count": 12, + "merge_request_diffs_synced_count": 9, + "merge_request_diffs_failed_count": 3, "package_files_count": 10, "package_files_checksummed_count": 10, "package_files_checksum_failed_count": 0, diff --git a/doc/development/geo/framework.md b/doc/development/geo/framework.md index c2aa0ff233d..e9ae74ae6b9 100644 --- a/doc/development/geo/framework.md +++ b/doc/development/geo/framework.md @@ -526,7 +526,7 @@ Metrics are gathered by `Geo::MetricsUpdateWorker`, persisted in `GET /geo_nodes/status` example response in `doc/api/geo_nodes.md`. 1. Add the same fields to `GET /geo_nodes/status` example response in - `doc/api/geo_nodes.md`. + `ee/spec/fixtures/api/schemas/public_api/v4/geo_node_status.json`. 1. Add fields `geo_widgets`, `geo_widgets_checksummed`, `geo_widgets_checksum_failed`, `geo_widgets_synced`, `geo_widgets_failed`, and `geo_widgets_registry` to diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 68f877cd3ac..828ed38194b 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -523,6 +523,15 @@ Source: - Search for `rate_limit_http_rate_per_minute` and `rate_limit_sessions_per_second` in [GitLab.com's current HAProxy settings](https://gitlab.com/gitlab-cookbooks/gitlab-haproxy/blob/master/attributes/default.rb). +### Pagination response headers + +For performance reasons, if a query returns more than 10,000 records, GitLab +doesn't return the following headers: + +- `X-Total`. +- `X-Total-Pages`. +- `rel="last"` `Link`. + ### Rack Attack initializer Details of rate limits enforced by [Rack Attack](../../security/rack_attack.md). diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index a240c367a42..9ec5df8cde9 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -190,8 +190,20 @@ excluded_attributes: - :stored_externally - :external_diff_store - :merge_request_id + - :verification_retry_at + - :verified_at + - :verification_retry_count + - :verification_checksum + - :verification_failure merge_request_diff_commits: - :merge_request_diff_id + merge_request_diff_detail: + - :merge_request_diff_id + - :verification_retry_at + - :verified_at + - :verification_retry_count + - :verification_checksum + - :verification_failure merge_request_diff_files: - :diff - :external_diff_offset diff --git a/spec/bin/feature_flag_spec.rb b/spec/bin/feature_flag_spec.rb index f85b8f22210..92d8fbddd44 100644 --- a/spec/bin/feature_flag_spec.rb +++ b/spec/bin/feature_flag_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' +require 'rspec-parameterized' load File.expand_path('../../bin/feature-flag', __dir__) @@ -11,25 +12,20 @@ RSpec.describe 'bin/feature-flag' do let(:argv) { %w[feature-flag-name -t development -g group::memory -i https://url -m http://url] } let(:options) { FeatureFlagOptionParser.parse(argv) } let(:creator) { described_class.new(options) } - let(:existing_flag) { File.join('config', 'feature_flags', 'development', 'existing-feature-flag.yml') } + let(:existing_flags) do + { 'existing-feature-flag' => File.join('config', 'feature_flags', 'development', 'existing-feature-flag.yml') } + end before do - # create a dummy feature flag - FileUtils.mkdir_p(File.dirname(existing_flag)) - File.write(existing_flag, '{}') + allow(creator).to receive(:all_feature_flag_names) { existing_flags } + allow(creator).to receive(:branch_name) { 'feature-branch' } + allow(creator).to receive(:editor) { nil } # ignore writes allow(File).to receive(:write).and_return(true) # ignore stdin allow($stdin).to receive(:gets).and_raise('EOF') - - # ignore Git commands - allow(creator).to receive(:branch_name) { 'feature-branch' } - end - - after do - FileUtils.rm_f(existing_flag) end subject { creator.execute } diff --git a/spec/factories/merge_request_diffs.rb b/spec/factories/merge_request_diffs.rb index 0c4c3244af5..8110ffd0877 100644 --- a/spec/factories/merge_request_diffs.rb +++ b/spec/factories/merge_request_diffs.rb @@ -2,12 +2,20 @@ FactoryBot.define do factory :merge_request_diff do - association :merge_request + association :merge_request, :without_merge_request_diff state { :collected } commits_count { 1 } base_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } + + trait :external do + external_diff { fixture_file_upload("spec/fixtures/doc_sample.txt", "plain/txt") } + stored_externally { true } + importing { true } # this avoids setting the state to 'empty' + end + + factory :external_merge_request_diff, traits: [:external] end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 6836d5d71f0..9fd6ebe783c 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -92,6 +92,16 @@ FactoryBot.define do target_branch { "feature_two" } end + trait(:without_merge_request_diff) do + after(:build) do |_| + MergeRequest.skip_callback(:create, :after, :ensure_merge_request_diff) + end + + after(:create) do |_| + MergeRequest.set_callback(:create, :after, :ensure_merge_request_diff) + end + end + trait :locked do state_id { MergeRequest.available_states[:locked] } end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 3ad3f610128..d6cfadd39bf 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -179,9 +179,12 @@ external_pull_requests: merge_request_diff: - merge_request - merge_request_diff_commits +- merge_request_diff_detail - merge_request_diff_files merge_request_diff_commits: - merge_request_diff +merge_request_diff_detail: +- merge_request_diff merge_request_diff_files: - merge_request_diff merge_request_context_commits: