Add latest changes from gitlab-org/gitlab@master

changelogs/unreleased/sh-fix-current-settings.yml

+---
+title: Fix application settings not working with pending migrations
+merge_request:
+author:
+type: fixed

config/initializers/rspec_profiling.rb

@@ -61,7 +61,7 @@ RspecProfiling.configure do |config|
     RspecProfiling::Run.prepend(RspecProfilingExt::Run)
     config.collector = RspecProfilingExt::Collectors::CSVWithTimestamps
     config.csv_path = -> do
-      prefix = "#{ENV['CI_JOB_NAME']}-".tr(' ', '-') if ENV['CI_JOB_NAME']
+      prefix = "#{ENV['CI_JOB_NAME']}-".gsub(/[ \/]/, '-') if ENV['CI_JOB_NAME']
       "rspec_profiling/#{prefix}#{Time.now.to_i}-#{SecureRandom.hex(8)}-rspec-data.csv"
     end
   end

lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml

 .auto-deploy:
-  image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v0.9.1"
+  image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v0.9.3"
 review:
   extends: .auto-deploy

lib/gitlab/current_settings.rb

@@ -50,7 +50,7 @@ module Gitlab
         # need to be added to the application settings. To prevent Rake tasks
         # and other callers from failing, use any loaded settings and return
         # defaults for missing columns.
-        if ActiveRecord::Base.connection.migration_context.needs_migration?
+        if Gitlab::Runtime.rake? && ActiveRecord::Base.connection.migration_context.needs_migration?
           db_attributes = current_settings&.attributes || {}
           fake_application_settings(db_attributes)
         elsif current_settings.present?

lib/gitlab/database/batch_count.rb

+# frozen_string_literal: true
+
+# For large tables, PostgreSQL can take a long time to count rows due to MVCC.
+# Implements a distinct and ordinary batch counter
+# Needs indexes on the column below to calculate max, min and range queries
+# For larger tables just set use higher batch_size with index optimization
+# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22705
+# Examples:
+#  extend ::Gitlab::Database::BatchCount
+#  batch_count(User.active)
+#  batch_count(::Clusters::Cluster.aws_installed.enabled, :cluster_id)
+#  batch_distinct_count(::Project, :creator_id)
+module Gitlab
+  module Database
+    module BatchCount
+      def batch_count(relation, column = nil, batch_size: nil)
+        BatchCounter.new(relation, column: column).count(batch_size: batch_size)
+      end
+
+      def batch_distinct_count(relation, column = nil, batch_size: nil)
+        BatchCounter.new(relation, column: column).count(mode: :distinct, batch_size: batch_size)
+      end
+
+      class << self
+        include BatchCount
+      end
+    end
+
+    class BatchCounter
+      FALLBACK = -1
+      MIN_REQUIRED_BATCH_SIZE = 2_000
+      MAX_ALLOWED_LOOPS = 10_000
+      SLEEP_TIME_IN_SECONDS = 0.01 # 10 msec sleep
+      # Each query should take <<500ms https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22705
+      DEFAULT_DISTINCT_BATCH_SIZE = 100_000
+      DEFAULT_BATCH_SIZE = 10_000
+
+      def initialize(relation, column: nil)
+        @relation = relation
+        @column = column || relation.primary_key
+      end
+
+      def unwanted_configuration?(finish, batch_size, start)
+        batch_size <= MIN_REQUIRED_BATCH_SIZE ||
+          (finish - start) / batch_size >= MAX_ALLOWED_LOOPS ||
+          start > finish
+      end
+
+      def count(batch_size: nil, mode: :itself)
+        raise 'BatchCount can not be run inside a transaction' if ActiveRecord::Base.connection.transaction_open?
+        raise "The mode #{mode.inspect} is not supported" unless [:itself, :distinct].include?(mode)
+
+        # non-distinct have better performance
+        batch_size ||= mode == :distinct ? DEFAULT_BATCH_SIZE : DEFAULT_DISTINCT_BATCH_SIZE
+
+        start = @relation.minimum(@column) || 0
+        finish = @relation.maximum(@column) || 0
+
+        raise "Batch counting expects positive values only for #{@column}" if start < 0 || finish < 0
+        return FALLBACK if unwanted_configuration?(finish, batch_size, start)
+
+        counter = 0
+        batch_start = start
+
+        while batch_start <= finish
+          begin
+            counter += batch_fetch(batch_start, batch_start + batch_size, mode)
+            batch_start += batch_size
+          rescue ActiveRecord::QueryCanceled
+            # retry with a safe batch size & warmer cache
+            if batch_size >= 2 * MIN_REQUIRED_BATCH_SIZE
+              batch_size /= 2
+            else
+              return FALLBACK
+            end
+          end
+          sleep(SLEEP_TIME_IN_SECONDS)
+        end
+
+        counter
+      end
+
+      def batch_fetch(start, finish, mode)
+        # rubocop:disable GitlabSecurity/PublicSend
+        @relation.select(@column).public_send(mode).where(@column => start..(finish - 1)).count
+      end
+    end
+  end
+end

lib/gitlab/diff/diff_refs.rb

@@ -37,7 +37,7 @@ module Gitlab
       # We have `base_sha` directly available on `DiffRefs` because it's faster#
       # than having to look it up in the repo every time.
       def complete?
-        start_sha && head_sha
+        start_sha.present? && head_sha.present?
       end
       def compare_in(project)

lib/gitlab/diff/formatters/image_formatter.rb

@@ -23,7 +23,7 @@ module Gitlab
         end
         def complete?
-          x && y && width && height
+          [x, y, width, height].all?(&:present?)
         end
         def to_h

lib/gitlab/diff/formatters/text_formatter.rb

@@ -19,7 +19,7 @@ module Gitlab
         end
         def complete?
-          old_line || new_line
+          old_line.present? || new_line.present?
         end
         def to_h

lib/gitlab/usage_data.rb

@@ -67,8 +67,8 @@ module Gitlab
             clusters_disabled: count(::Clusters::Cluster.disabled),
             project_clusters_disabled: count(::Clusters::Cluster.disabled.project_type),
             group_clusters_disabled: count(::Clusters::Cluster.disabled.group_type),
-            clusters_platforms_eks: count(::Clusters::Cluster.aws_installed.enabled),
-            clusters_platforms_gke: count(::Clusters::Cluster.gcp_installed.enabled),
+            clusters_platforms_eks: count(::Clusters::Cluster.aws_installed.enabled, batch: false),
+            clusters_platforms_gke: count(::Clusters::Cluster.gcp_installed.enabled, batch: false),
             clusters_platforms_user: count(::Clusters::Cluster.user_provided.enabled),
             clusters_applications_helm: count(::Clusters::Applications::Helm.available),
             clusters_applications_ingress: count(::Clusters::Applications::Ingress.available),
@@ -85,7 +85,7 @@ module Gitlab
             issues: count(Issue),
             issues_created_from_gitlab_error_tracking_ui: count(SentryIssue),
             issues_with_associated_zoom_link: count(ZoomMeeting.added_to_issue),
-            issues_using_zoom_quick_actions: count(ZoomMeeting.select(:issue_id).distinct),
+            issues_using_zoom_quick_actions: count(ZoomMeeting.select(:issue_id).distinct, batch: false),
             issues_with_embedded_grafana_charts_approx: ::Gitlab::GrafanaEmbedUsageData.issue_count,
             incident_issues: count(::Issue.authored(::User.alert_bot)),
             keys: count(Key),
@@ -99,7 +99,7 @@ module Gitlab
             projects_imported_from_github: count(Project.where(import_type: 'github')),
             projects_with_repositories_enabled: count(ProjectFeature.where('repository_access_level > ?', ProjectFeature::DISABLED)),
             projects_with_error_tracking_enabled: count(::ErrorTracking::ProjectErrorTrackingSetting.where(enabled: true)),
-            projects_with_alerts_service_enabled: count(AlertsService.active),
+            projects_with_alerts_service_enabled: count(AlertsService.active, batch: false),
             protected_branches: count(ProtectedBranch),
             releases: count(Release),
             remote_mirrors: count(RemoteMirror),
@@ -181,7 +181,7 @@ module Gitlab
       # rubocop: disable CodeReuse/ActiveRecord
       def services_usage
-        service_counts = count(Service.active.where(template: false).where.not(type: 'JiraService').group(:type), fallback: Hash.new(-1))
+        service_counts = count(Service.active.where(template: false).where.not(type: 'JiraService').group(:type), fallback: Hash.new(-1), batch: false)
         results = Service.available_services_names.each_with_object({}) do |service_name, response|
           response["projects_#{service_name}_active".to_sym] = service_counts["#{service_name}_service".camelize] || 0
@@ -217,9 +217,9 @@ module Gitlab
           results[:projects_jira_server_active] += counts[:server].count if counts[:server]
           results[:projects_jira_cloud_active] += counts[:cloud].count if counts[:cloud]
           if results[:projects_jira_active] == -1
-            results[:projects_jira_active] = count(services)
+            results[:projects_jira_active] = count(services, batch: false)
           else
-            results[:projects_jira_active] += count(services)
+            results[:projects_jira_active] += count(services, batch: false)
           end
         end
@@ -231,8 +231,22 @@ module Gitlab
         {} # augmented in EE
       end
-      def count(relation, fallback: -1)
-        relation.count
+      def count(relation, column = nil, fallback: -1, batch: true)
+        if batch && Feature.enabled?(:usage_ping_batch_counter)
+          Gitlab::Database::BatchCount.batch_count(relation, column)
+        else
+          relation.count
+        end
+      rescue ActiveRecord::StatementInvalid
+        fallback
+      end
+
+      def distinct_count(relation, column = nil, fallback: -1, batch: true)
+        if batch && Feature.enabled?(:usage_ping_batch_counter)
+          Gitlab::Database::BatchCount.batch_distinct_count(relation, column)
+        else
+          relation.distinct_count_by(column)
+        end
       rescue ActiveRecord::StatementInvalid
         fallback
       end

locale/gitlab.pot

@@ -15795,9 +15795,6 @@ msgstr ""
 msgid "Related Merged Requests"
 msgstr ""
-msgid "Related issues"
-msgstr ""
-
 msgid "Related merge requests"
 msgstr ""

spec/controllers/admin/application_settings_controller_spec.rb

@@ -16,6 +16,7 @@ describe Admin::ApplicationSettingsController do
   describe 'GET #usage_data with no access' do
     before do
+      allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false)
       sign_in(user)
     end
@@ -28,6 +29,7 @@ describe Admin::ApplicationSettingsController do
   describe 'GET #usage_data' do
     before do
+      allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false)
       sign_in(admin)
     end

spec/factories/diff_position.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :diff_position, class: 'Gitlab::Diff::Position' do
+    skip_create # non-model factories (i.e. without #save)
+
+    transient do
+      file { 'path/to/file' }
+
+      # Allow diff to be passed as a single object.
+      diff_refs do
+        ::Gitlab::Diff::DiffRefs.new(
+          base_sha: Digest::SHA1.hexdigest(SecureRandom.hex),
+          head_sha: Digest::SHA1.hexdigest(SecureRandom.hex),
+          start_sha: Digest::SHA1.hexdigest(SecureRandom.hex)
+        )
+      end
+    end
+
+    old_path { file }
+    new_path { file }
+
+    base_sha  { diff_refs&.base_sha }
+    head_sha  { diff_refs&.head_sha }
+    start_sha { diff_refs&.start_sha }
+
+    initialize_with { new(attributes) }
+
+    trait :moved do
+      new_path { 'path/to/new.file' }
+    end
+
+    factory :text_diff_position do
+      position_type { 'text' }
+      old_line { 10 }
+      new_line { 10 }
+
+      trait :added do
+        old_line { nil }
+      end
+    end
+
+    factory :image_diff_position do
+      position_type { 'image' }
+      x { 1 }
+      y { 1 }
+      width { 10 }
+      height { 10 }
+    end
+  end
+end

spec/factories/notes.rb

@@ -58,24 +58,20 @@ FactoryBot.define do
       end
       position do
-        Gitlab::Diff::Position.new(
-          old_path: "files/ruby/popen.rb",
-          new_path: "files/ruby/popen.rb",
-          old_line: nil,
-          new_line: line_number,
-          diff_refs: diff_refs
-        )
+        build(:text_diff_position,
+              file: "files/ruby/popen.rb",
+              old_line: nil,
+              new_line: line_number,
+              diff_refs: diff_refs)
       end
       trait :folded_position do
         position do
-          Gitlab::Diff::Position.new(
-            old_path: "files/ruby/popen.rb",
-            new_path: "files/ruby/popen.rb",
-            old_line: 1,
-            new_line: 1,
-            diff_refs: diff_refs
-          )
+          build(:text_diff_position,
+                file: "files/ruby/popen.rb",
+                old_line: 1,
+                new_line: 1,
+                diff_refs: diff_refs)
         end
       end
@@ -86,16 +82,9 @@ FactoryBot.define do
       factory :image_diff_note_on_merge_request do
         position do
-          Gitlab::Diff::Position.new(
-            old_path: "files/images/any_image.png",
-            new_path: "files/images/any_image.png",
-            width: 10,
-            height: 10,
-            x: 1,
-            y: 1,
-            diff_refs: diff_refs,
-            position_type: "image"
-          )
+          build(:image_diff_position,
+                file: "files/images/any_image.png",
+                diff_refs: diff_refs)
         end
       end
     end
@@ -109,9 +98,8 @@ FactoryBot.define do
       end
       position do
-        Gitlab::Diff::Position.new(
-          old_path: "files/ruby/popen.rb",
-          new_path: "files/ruby/popen.rb",
+        build(:text_diff_position,
+          file: "files/ruby/popen.rb",
           old_line: nil,
           new_line: line_number,
           diff_refs: diff_refs

spec/features/admin/admin_settings_spec.rb

@@ -326,6 +326,8 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
       end
       it 'loads usage ping payload on click', :js do
+        allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false)
+
         expect(page).to have_button 'Preview payload'
         find('.js-usage-ping-payload-trigger').click

spec/features/merge_request/user_creates_image_diff_notes_spec.rb

@@ -48,29 +48,11 @@ describe 'Merge request > User creates image diff notes', :js do
       let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
       let(:note1_position) do
-        Gitlab::Diff::Position.new(
-          old_path: path,
-          new_path: path,
-          width: 100,
-          height: 100,
-          x: 10,
-          y: 10,
-          position_type: "image",
-          diff_refs: commit.diff_refs
-        )
+        build(:image_diff_position, file: path, diff_refs: commit.diff_refs)
       end
       let(:note2_position) do
-        Gitlab::Diff::Position.new(
-          old_path: path,
-          new_path: path,
-          width: 100,
-          height: 100,
-          x: 20,
-          y: 20,
-          position_type: "image",
-          diff_refs: commit.diff_refs
-        )
+        build(:image_diff_position, file: path, diff_refs: commit.diff_refs)
       end
       let!(:note1) { create(:diff_note_on_commit, commit_id: commit.id, project: project, position: note1_position, note: 'my note 1') }
@@ -93,16 +75,7 @@ describe 'Merge request > User creates image diff notes', :js do
   %w(inline parallel).each do |view|
     context "#{view} view" do
       let(:position) do
-        Gitlab::Diff::Position.new(
-          old_path: path,
-          new_path: path,
-          width: 100,
-          height: 100,
-          x: 1,
-          y: 1,
-          position_type: "image",
-          diff_refs: merge_request.diff_refs
-        )
+        build(:image_diff_position, file: path, diff_refs: merge_request.diff_refs)
       end
       let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) }
@@ -167,16 +140,7 @@ describe 'Merge request > User creates image diff notes', :js do
     let(:path)          { "files/images/ee_repo_logo.png" }
     let(:position) do
-      Gitlab::Diff::Position.new(
-        old_path: path,
-        new_path: path,
-        width: 100,
-        height: 100,
-        x: 50,
-        y: 50,
-        position_type: "image",
-        diff_refs: merge_request.diff_refs
-      )
+      build(:image_diff_position, file: path, diff_refs: merge_request.diff_refs)
     end
     before do

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -10,13 +10,9 @@ describe 'Merge request > User resolves diff notes and threads', :js do
   let!(:note)         { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "| Markdown | Table |\n|-------|---------|\n| first | second |") }
   let(:path)          { "files/ruby/popen.rb" }
   let(:position) do
-    Gitlab::Diff::Position.new(
-      old_path: path,
-      new_path: path,
-      old_line: nil,
-      new_line: 9,
-      diff_refs: merge_request.diff_refs
-    )
+    build(:text_diff_position,
+          file: path, old_line: nil, new_line: 9,
+          diff_refs: merge_request.diff_refs)
   end
   before do

spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb

@@ -13,20 +13,16 @@ describe 'Merge request > User resolves outdated diff discussions', :js do
   let(:current_diff_refs) { merge_request.diff_refs }
   let(:outdated_position) do
-    Gitlab::Diff::Position.new(
-      old_path: 'files/csv/Book1.csv',
-      new_path: 'files/csv/Book1.csv',
-      old_line: nil,
+    build(:text_diff_position, :added,
+      file: 'files/csv/Book1.csv',
       new_line: 9,
       diff_refs: outdated_diff_refs
     )
   end
   let(:current_position) do
-    Gitlab::Diff::Position.new(
-      old_path: 'files/csv/Book1.csv',
-      new_path: 'files/csv/Book1.csv',
-      old_line: nil,
+    build(:text_diff_position, :added,
+      file: 'files/csv/Book1.csv',
       new_line: 1,
       diff_refs: current_diff_refs
     )

spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb

@@ -10,10 +10,8 @@ describe 'Merge request > User sees avatars on diff notes', :js do
   let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: 'Bug NS-04') }
   let(:path)          { 'files/ruby/popen.rb' }
   let(:position) do
-    Gitlab::Diff::Position.new(
-      old_path: path,
-      new_path: path,
-      old_line: nil,
+    build(:text_diff_position, :added,
+      file: path,
       new_line: 9,
       diff_refs: merge_request.diff_refs
     )

spec/features/merge_request/user_sees_discussions_spec.rb

@@ -18,10 +18,8 @@ describe 'Merge request > User sees threads', :js do
     let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion }
     let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
     let(:outdated_position) do
-      Gitlab::Diff::Position.new(
-        old_path: "files/ruby/popen.rb",
-        new_path: "files/ruby/popen.rb",
-        old_line: nil,
+      build(:text_diff_position, :added,
+        file: "files/ruby/popen.rb",
         new_line: 9,
         diff_refs: outdated_diff_refs
       )

spec/features/merge_request/user_sees_versions_spec.rb

@@ -86,10 +86,8 @@ describe 'Merge request > User sees versions', :js do
     it 'shows comments that were last relevant at that version' do
       expect(page).to have_content '5 files'
-      position = Gitlab::Diff::Position.new(
-        old_path: ".gitmodules",
-        new_path: ".gitmodules",
-        old_line: nil,
+      position = build(:text_diff_position, :added,
+        file: ".gitmodules",
         new_line: 4,
         diff_refs: merge_request_diff1.diff_refs
       )
@@ -136,9 +134,8 @@ describe 'Merge request > User sees versions', :js do
       expect(additions_content).to eq '15'
       expect(deletions_content).to eq '6'
-      position = Gitlab::Diff::Position.new(
-        old_path: ".gitmodules",
-        new_path: ".gitmodules",
+      position = build(:text_diff_position,
+        file: ".gitmodules",
         old_line: 4,
         new_line: 4,
         diff_refs: merge_request_diff3.compare_with(merge_request_diff1.head_commit_sha).diff_refs