Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/boards/stores/actions.js

@@ -173,8 +173,9 @@ export default {
 
   addList: ({ commit, dispatch, getters }, list) => {
     commit(types.RECEIVE_ADD_LIST_SUCCESS, updateListPosition(list));
+
     dispatch('fetchItemsForList', {
-      listId: getters.getListByTitle(ListTypeTitles.backlog).id,
+      listId: getters.getListByTitle(ListTypeTitles.backlog)?.id,
     });
   },
 
@@ -294,7 +295,7 @@ export default {
             commit(types.REMOVE_LIST_FAILURE, listsBackup);
           } else {
             dispatch('fetchItemsForList', {
-              listId: getters.getListByTitle(ListTypeTitles.backlog).id,
+              listId: getters.getListByTitle(ListTypeTitles.backlog)?.id,
             });
           }
         },
@@ -305,6 +306,8 @@ export default {
   },
 
   fetchItemsForList: ({ state, commit }, { listId, fetchNext = false }) => {
+    if (!listId) return null;
+
     if (!fetchNext) {
       commit(types.RESET_ITEMS_FOR_LIST, listId);
     }

app/controllers/admin/cohorts_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Admin::CohortsController < Admin::ApplicationController
-  include Analytics::UniqueVisitsHelper
+  include RedisTracking
 
   feature_category :devops_reports
 
@@ -21,6 +21,6 @@ class Admin::CohortsController < Admin::ApplicationController
   end
 
   def track_cohorts_visit
-    track_visit('i_analytics_cohorts') if trackable_html_request?
+    track_unique_redis_hll_event('i_analytics_cohorts') if trackable_html_request?
   end
 end

app/controllers/admin/usage_trends_controller.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 class Admin::UsageTrendsController < Admin::ApplicationController
-  include Analytics::UniqueVisitsHelper
+  include RedisTracking
 
-  track_unique_visits :index, target_id: 'i_analytics_instance_statistics'
+  track_redis_hll_event :index, name: 'i_analytics_instance_statistics'
 
   feature_category :devops_reports
 

app/controllers/dashboard/todos_controller.rb

@@ -3,7 +3,6 @@
 class Dashboard::TodosController < Dashboard::ApplicationController
   include ActionView::Helpers::NumberHelper
   include PaginatedCollection
-  include Analytics::UniqueVisitsHelper
 
   before_action :authorize_read_project!, only: :index
   before_action :authorize_read_group!, only: :index

app/controllers/projects/cycle_analytics_controller.rb

@@ -4,12 +4,12 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
   include ActionView::Helpers::DateHelper
   include ActionView::Helpers::TextHelper
   include CycleAnalyticsParams
-  include Analytics::UniqueVisitsHelper
   include GracefulTimeoutHandling
+  include RedisTracking
 
   before_action :authorize_read_cycle_analytics!
 
-  track_unique_visits :show, target_id: 'p_analytics_valuestream'
+  track_redis_hll_event :show, name: 'p_analytics_valuestream'
 
   feature_category :planning_analytics
 

app/controllers/projects/graphs_controller.rb

@@ -2,14 +2,14 @@
 
 class Projects::GraphsController < Projects::ApplicationController
   include ExtractsPath
-  include Analytics::UniqueVisitsHelper
+  include RedisTracking
 
   # Authorize
   before_action :require_non_empty_project
   before_action :assign_ref_vars
   before_action :authorize_read_repository_graphs!
 
-  track_unique_visits :charts, target_id: 'p_analytics_repo'
+  track_redis_hll_event :charts, name: 'p_analytics_repo'
 
   feature_category :source_code_management
 

app/controllers/projects/pipelines_controller.rb

@@ -2,7 +2,7 @@
 
 class Projects::PipelinesController < Projects::ApplicationController
   include ::Gitlab::Utils::StrongMemoize
-  include Analytics::UniqueVisitsHelper
+  include RedisTracking
 
   before_action :disable_query_limiting, only: [:create, :retry]
   before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables]
@@ -24,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController
 
   around_action :allow_gitaly_ref_name_caching, only: [:index, :show]
 
-  track_unique_visits :charts, target_id: 'p_analytics_pipelines'
+  track_redis_hll_event :charts, name: 'p_analytics_pipelines'
 
   wrap_parameters Ci::Pipeline
 

app/helpers/analytics/unique_visits_helper.rb

@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-module Analytics
-  module UniqueVisitsHelper
-    include Gitlab::Tracking::Helpers
-    extend ActiveSupport::Concern
-
-    def visitor_id
-      return cookies[:visitor_id] if cookies[:visitor_id].present?
-      return unless current_user
-
-      uuid = SecureRandom.uuid
-      cookies[:visitor_id] = { value: uuid, expires: 24.months }
-      uuid
-    end
-
-    def track_visit(target_id)
-      return unless visitor_id
-
-      Gitlab::Analytics::UniqueVisits.new.track_visit(target_id, values: visitor_id)
-    end
-
-    class_methods do
-      def track_unique_visits(controller_actions, target_id:)
-        after_action only: controller_actions, if: -> { trackable_html_request? } do
-          track_visit(target_id)
-        end
-      end
-    end
-  end
-end

app/models/concerns/atomic_internal_id.rb

@@ -159,9 +159,8 @@ module AtomicInternalId
     # Defines class methods:
     #
     # - with_{scope}_{column}_supply
-    #   This method can be used to allocate a block of IID values during
-    #   bulk operations (importing/copying, etc). This can be more efficient
-    #   than creating instances one-by-one.
+    #   This method can be used to allocate a stream of IID values during
+    #   bulk operations (importing/copying, etc).
     #
     #   Pass in a block that receives a `Supply` instance. To allocate a new
     #   IID value, call `Supply#next_value`.
@@ -181,14 +180,8 @@ module AtomicInternalId
         scope_attrs = ::AtomicInternalId.scope_attrs(scope_value)
         usage = ::AtomicInternalId.scope_usage(self)
 
-        generator = InternalId::InternalIdGenerator.new(subject, scope_attrs, usage, init)
-
-        generator.with_lock do
-          supply = Supply.new(generator.record.last_value)
+        supply = Supply.new(-> { InternalId.generate_next(subject, scope_attrs, usage, init) })
         block.call(supply)
-        ensure
-          generator.track_greatest(supply.current_value) if supply
-        end
       end
     end
   end
@@ -236,14 +229,14 @@ module AtomicInternalId
   end
 
   class Supply
-    attr_reader :current_value
+    attr_reader :generator
 
-    def initialize(start_value)
-      @current_value = start_value
+    def initialize(generator)
+      @generator = generator
     end
 
     def next_value
-      @current_value += 1
+      @generator.call
     end
   end
 end

app/models/internal_id.rb

@@ -16,7 +16,7 @@
 # * Add `usage` value to enum
 # * (Optionally) add columns to `internal_ids` if needed for scope.
 class InternalId < ApplicationRecord
-  include Gitlab::Utils::StrongMemoize
+  extend Gitlab::Utils::StrongMemoize
 
   belongs_to :project
   belongs_to :namespace
@@ -25,6 +25,10 @@ class InternalId < ApplicationRecord
 
   validates :usage, presence: true
 
+  scope :filter_by, -> (scope, usage) do
+    where(**scope, usage: usage)
+  end
+
   # Increments #last_value and saves the record
   #
   # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
@@ -53,18 +57,15 @@ class InternalId < ApplicationRecord
 
   class << self
     def track_greatest(subject, scope, usage, new_value, init)
-      InternalIdGenerator.new(subject, scope, usage, init)
-        .track_greatest(new_value)
+      build_generator(subject, scope, usage, init).track_greatest(new_value)
     end
 
     def generate_next(subject, scope, usage, init)
-      InternalIdGenerator.new(subject, scope, usage, init)
-        .generate
+      build_generator(subject, scope, usage, init).generate
     end
 
     def reset(subject, scope, usage, value)
-      InternalIdGenerator.new(subject, scope, usage)
-        .reset(value)
+      build_generator(subject, scope, usage).reset(value)
     end
 
     # Flushing records is generally safe in a sense that those
@@ -77,11 +78,36 @@ class InternalId < ApplicationRecord
 
       where(filter).delete_all
     end
+
+    def internal_id_transactions_increment(operation:, usage:)
+      self.internal_id_transactions_total.increment(
+        operation: operation,
+        usage: usage.to_s,
+        in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s
+      )
+    end
+
+    def internal_id_transactions_total
+      strong_memoize(:internal_id_transactions_total) do
+        name = :gitlab_internal_id_transactions_total
+        comment = 'Counts all the internal ids happening within transaction'
+
+        Gitlab::Metrics.counter(name, comment)
+      end
+    end
+
+    private
+
+    def build_generator(subject, scope, usage, init = nil)
+      if Feature.enabled?(:generate_iids_without_explicit_locking)
+        ImplicitlyLockingInternalIdGenerator.new(subject, scope, usage, init)
+      else
+        InternalIdGenerator.new(subject, scope, usage, init)
+      end
+    end
   end
 
   class InternalIdGenerator
-    extend Gitlab::Utils::StrongMemoize
-
     # Generate next internal id for a given scope and usage.
     #
     # For currently supported usages, see #usage enum.
@@ -117,7 +143,7 @@ class InternalId < ApplicationRecord
     # init: Block that gets called to initialize InternalId record if not present
     #       Make sure to not throw exceptions in the absence of records (if this is expected).
     def generate
-      self.class.internal_id_transactions_increment(operation: :generate, usage: usage)
+      InternalId.internal_id_transactions_increment(operation: :generate, usage: usage)
 
       subject.transaction do
         # Create a record in internal_ids if one does not yet exist
@@ -134,7 +160,7 @@ class InternalId < ApplicationRecord
     def reset(value)
       return false unless value
 
-      self.class.internal_id_transactions_increment(operation: :reset, usage: usage)
+      InternalId.internal_id_transactions_increment(operation: :reset, usage: usage)
 
       updated =
         InternalId
@@ -149,8 +175,9 @@ class InternalId < ApplicationRecord
     # and set its new_value if it is higher than the current last_value
     #
     # Note this will acquire a ROW SHARE lock on the InternalId record
+
     def track_greatest(new_value)
-      self.class.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
+      InternalId.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
 
       subject.transaction do
         record.track_greatest_and_save!(new_value)
@@ -162,7 +189,7 @@ class InternalId < ApplicationRecord
     end
 
     def with_lock(&block)
-      self.class.internal_id_transactions_increment(operation: :with_lock, usage: usage)
+      InternalId.internal_id_transactions_increment(operation: :with_lock, usage: usage)
 
       record.with_lock(&block)
     end
@@ -199,22 +226,118 @@ class InternalId < ApplicationRecord
     rescue ActiveRecord::RecordNotUnique
       lookup
     end
-
-    def self.internal_id_transactions_increment(operation:, usage:)
-      self.internal_id_transactions_total.increment(
-        operation: operation,
-        usage: usage.to_s,
-        in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s
-      )
   end
 
-    def self.internal_id_transactions_total
-      strong_memoize(:internal_id_transactions_total) do
-        name = :gitlab_internal_id_transactions_total
-        comment = 'Counts all the internal ids happening within transaction'
+  class ImplicitlyLockingInternalIdGenerator
+    # Generate next internal id for a given scope and usage.
+    #
+    # For currently supported usages, see #usage enum.
+    #
+    # The method implements a locking scheme that has the following properties:
+    # 1) Generated sequence of internal ids is unique per (scope and usage)
+    # 2) The method is thread-safe and may be used in concurrent threads/processes.
+    # 3) The generated sequence is gapless.
+    # 4) In the absence of a record in the internal_ids table, one will be created
+    #    and last_value will be calculated on the fly.
+    #
+    # subject: The instance or class we're generating an internal id for.
+    # scope: Attributes that define the scope for id generation.
+    #        Valid keys are `project/project_id` and `namespace/namespace_id`.
+    # usage: Symbol to define the usage of the internal id, see InternalId.usages
+    # init: Proc that accepts the subject and the scope and returns Integer|NilClass
+    attr_reader :subject, :scope, :scope_attrs, :usage, :init
 
-        Gitlab::Metrics.counter(name, comment)
-      end
+    def initialize(subject, scope, usage, init = nil)
+      @subject = subject
+      @scope = scope
+      @usage = usage
+      @init = init
+
+      raise ArgumentError, 'Scope is not well-defined, need at least one column for scope (given: 0)' if scope.empty?
+
+      unless InternalId.usages.has_key?(usage.to_s)
+        raise ArgumentError, "Usage '#{usage}' is unknown. Supported values are #{InternalId.usages.keys} from InternalId.usages"
+      end
+    end
+
+    # Generates next internal id and returns it
+    # init: Block that gets called to initialize InternalId record if not present
+    #       Make sure to not throw exceptions in the absence of records (if this is expected).
+    def generate
+      InternalId.internal_id_transactions_increment(operation: :generate, usage: usage)
+
+      next_iid = update_record!(subject, scope, usage, arel_table[:last_value] + 1)
+
+      return next_iid if next_iid
+
+      create_record!(subject, scope, usage, init) do |iid|
+        iid.last_value += 1
+      end
+    rescue ActiveRecord::RecordNotUnique
+      retry
+    end
+
+    # Reset tries to rewind to `value-1`. This will only succeed,
+    # if `value` stored in database is equal to `last_value`.
+    # value: The expected last_value to decrement
+    def reset(value)
+      return false unless value
+
+      InternalId.internal_id_transactions_increment(operation: :reset, usage: usage)
+
+      iid = update_record!(subject, scope.merge(last_value: value), usage, arel_table[:last_value] - 1)
+      iid == value - 1
+    end
+
+    # Create a record in internal_ids if one does not yet exist
+    # and set its new_value if it is higher than the current last_value
+    def track_greatest(new_value)
+      InternalId.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
+
+      function = Arel::Nodes::NamedFunction.new('GREATEST', [
+        arel_table[:last_value],
+        new_value.to_i
+      ])
+
+      next_iid = update_record!(subject, scope, usage, function)
+      return next_iid if next_iid
+
+      create_record!(subject, scope, usage, init) do |object|
+        object.last_value = [object.last_value, new_value].max
+      end
+    rescue ActiveRecord::RecordNotUnique
+      retry
+    end
+
+    private
+
+    def update_record!(subject, scope, usage, new_value)
+      stmt = Arel::UpdateManager.new
+      stmt.table(arel_table)
+      stmt.set(arel_table[:last_value] => new_value)
+      stmt.wheres = InternalId.filter_by(scope, usage).arel.constraints
+
+      ActiveRecord::Base.connection.insert(stmt, 'Update InternalId', 'last_value')
+    end
+
+    def create_record!(subject, scope, usage, init)
+      raise ArgumentError, 'Cannot initialize without init!' unless init
+
+      instance = subject.is_a?(::Class) ? nil : subject
+
+      subject.transaction(requires_new: true) do
+        last_value = init.call(instance, scope) || 0
+
+        internal_id = InternalId.create!(**scope, usage: usage, last_value: last_value) do |subject|
+          yield subject if block_given?
+        end
+
+        internal_id.last_value
+      end
+    end
+
+    def arel_table
+      InternalId.arel_table
     end
   end
 end

config/feature_flags/development/generate_iids_without_explicit_locking.yml

@@ -0,0 +1,8 @@
+---
+name: generate_iids_without_explicit_locking
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65590
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335431
+milestone: '14.2'
+type: development
+group: group::database
+default_enabled: false

doc/topics/set_up_organization.md

@@ -10,6 +10,7 @@ Configure your organization and its users. Determine user roles
 and give everyone access to the projects they need.
 
 - [Members](../user/project/members/index.md)
+- [Workspace](../user/workspace/index.md) _(Coming soon)_
 - [Groups](../user/group/index.md)
 - [User account options](../user/profile/index.md)
 - [SSH keys](../ssh/index.md)

doc/user/workspace/index.md

@@ -0,0 +1,34 @@
+---
+stage: Manage
+group: Access
+info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
+---
+
+# Workspace
+
+Workspace will be the top-level [namespace](../group/index.md#namespaces) for you to manage
+everything GitLab, including:
+
+- Defining and applying settings to all of your groups, subgroups, and projects.
+- Aggregating data from all your groups, subgroups, and projects.
+
+Workspace will take many of the features from the
+[Admin Area](../admin_area/index.md), and there will be one workspace per:
+
+- Instance, for self-managed instances.
+- Namespace, for GitLab.com.
+
+NOTE:
+Workspace is currently in development.
+
+## Concept previews
+
+The following provide a preview to the Workspace concept.
+
+![Workspace Overview](img/1.1-Instance_overview.png)
+
+![Groups Overview](img/1.2-Groups_overview.png)
+
+![Admin Overview](img/1.3-Admin.png)
+
+![Admin Overview](img/Admin_Settings.png)

lib/gitlab/analytics/unique_visits.rb

@@ -3,10 +3,6 @@
 module Gitlab
   module Analytics
     class UniqueVisits
-      def track_visit(*args, **kwargs)
-        Gitlab::UsageDataCounters::HLLRedisCounter.track_event(*args, **kwargs)
-      end
-
       # Returns number of unique visitors for given targets in given time frame
       #
       # @param [String, Array[<String>]] targets ids of targets to count visits on. Special case for :any

spec/frontend/boards/mock_data.js

@@ -291,7 +291,7 @@ export const setMockEndpoints = (opts = {}) => {
 
 export const mockList = {
   id: 'gid://gitlab/List/1',
-  title: 'Backlog',
+  title: 'Open',
   position: -Infinity,
   listType: 'backlog',
   collapsed: false,

spec/frontend/boards/stores/actions_spec.js

@@ -715,6 +715,19 @@ describe('fetchItemsForList', () => {
     [listId]: pageInfo,
   };
 
+  describe('when list id is undefined', () => {
+    it('does not call the query', async () => {
+      jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse);
+
+      await actions.fetchItemsForList(
+        { state, getters: () => {}, commit: () => {} },
+        { listId: undefined },
+      );
+
+      expect(gqlClient.query).toHaveBeenCalledTimes(0);
+    });
+  });
+
   it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_SUCCESS on success', (done) => {
     jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse);
 

spec/helpers/analytics/unique_visits_helper_spec.rb

@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-require "spec_helper"
-
-RSpec.describe Analytics::UniqueVisitsHelper do
-  include Devise::Test::ControllerHelpers
-
-  describe '#track_visit' do
-    let(:target_id) { 'p_analytics_valuestream' }
-    let(:current_user) { create(:user) }
-
-    it 'does not track visit if user is not logged in' do
-      expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
-
-      helper.track_visit(target_id)
-    end
-
-    it 'tracks visit if user is logged in' do
-      sign_in(current_user)
-
-      expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
-
-      helper.track_visit(target_id)
-    end
-
-    it 'tracks visit if user is not logged in, but has the cookie already' do
-      helper.request.cookies[:visitor_id] = { value: SecureRandom.uuid, expires: 24.months }
-
-      expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
-
-      helper.track_visit(target_id)
-    end
-  end
-end

spec/lib/gitlab/analytics/unique_visits_spec.rb

@@ -1,81 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state do
-  let(:unique_visits) { Gitlab::Analytics::UniqueVisits.new }
-  let(:target1_id) { 'g_analytics_contribution' }
-  let(:target2_id) { 'g_analytics_insights' }
-  let(:target3_id) { 'g_analytics_issues' }
-  let(:target4_id) { 'g_compliance_dashboard' }
-  let(:target5_id) { 'i_compliance_credential_inventory' }
-  let(:visitor1_id) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' }
-  let(:visitor2_id) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' }
-  let(:visitor3_id) { '34rfjuuy-ce56-sa35-ds34-dfer567dfrf2' }
-
-  around do |example|
-    # We need to freeze to a reference time
-    # because visits are grouped by the week number in the year
-    # Without freezing the time, the test may behave inconsistently
-    # depending on which day of the week test is run.
-    reference_time = Time.utc(2020, 6, 1)
-    travel_to(reference_time) { example.run }
-  end
-
-  describe '#track_visit' do
-    it 'tracks the unique weekly visits for targets' do
-      unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago)
-      unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago)
-      unique_visits.track_visit(target1_id, values: visitor2_id, time: 7.days.ago)
-
-      unique_visits.track_visit(target2_id, values: visitor2_id, time: 7.days.ago)
-      unique_visits.track_visit(target2_id, values: visitor1_id, time: 8.days.ago)
-      unique_visits.track_visit(target2_id, values: visitor1_id, time: 15.days.ago)
-
-      unique_visits.track_visit(target4_id, values: visitor3_id, time: 7.days.ago)
-
-      unique_visits.track_visit(target5_id, values: visitor3_id, time: 15.days.ago)
-      unique_visits.track_visit(target5_id, values: visitor2_id, time: 15.days.ago)
-
-      expect(unique_visits.unique_visits_for(targets: target1_id)).to eq(2)
-      expect(unique_visits.unique_visits_for(targets: target2_id)).to eq(1)
-      expect(unique_visits.unique_visits_for(targets: target4_id)).to eq(1)
-
-      expect(unique_visits.unique_visits_for(targets: target2_id, start_date: 15.days.ago)).to eq(1)
-
-      expect(unique_visits.unique_visits_for(targets: target3_id)).to eq(0)
-
-      expect(unique_visits.unique_visits_for(targets: target5_id, start_date: 15.days.ago)).to eq(2)
-
-      expect(unique_visits.unique_visits_for(targets: :analytics)).to eq(2)
-      expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 15.days.ago)).to eq(1)
-      expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 30.days.ago)).to eq(0)
-
-      expect(unique_visits.unique_visits_for(targets: :analytics, start_date: 4.weeks.ago, end_date: Date.current)).to eq(2)
-
-      expect(unique_visits.unique_visits_for(targets: :compliance)).to eq(1)
-      expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 15.days.ago)).to eq(2)
-      expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 30.days.ago)).to eq(0)
-
-      expect(unique_visits.unique_visits_for(targets: :compliance, start_date: 4.weeks.ago, end_date: Date.current)).to eq(2)
-    end
-
-    it 'sets the keys in Redis to expire automatically after 12 weeks' do
-      unique_visits.track_visit(target1_id, values: visitor1_id)
-
-      Gitlab::Redis::SharedState.with do |redis|
-        redis.scan_each(match: "{#{target1_id}}-*").each do |key|
-          expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks)
-        end
-      end
-    end
-
-    it 'raises an error if an invalid target id is given' do
-      invalid_target_id = "x_invalid"
-
-      expect do
-        unique_visits.track_visit(invalid_target_id, values: visitor1_id)
-      end.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent)
-    end
-  end
-end

spec/models/concerns/atomic_internal_id_spec.rb

@@ -240,18 +240,12 @@ RSpec.describe AtomicInternalId do
   end
 
   describe '.with_project_iid_supply' do
-    let(:iid) { 100 }
-
-    it 'wraps generate and track_greatest in a concurrency-safe lock' do
-      expect_next_instance_of(InternalId::InternalIdGenerator) do |g|
-        expect(g).to receive(:with_lock).and_call_original
-        expect(g.record).to receive(:last_value).and_return(iid)
-        expect(g).to receive(:track_greatest).with(iid + 4)
-      end
-
+    it 'supplies a stream of iid values' do
+      expect do
         ::Milestone.with_project_iid_supply(milestone.project) do |supply|
           4.times { supply.next_value }
         end
+      end.to change { InternalId.find_by(project: milestone.project, usage: :milestones)&.last_value.to_i }.by(4)
     end
   end
 end

spec/models/internal_id_spec.rb

@@ -39,6 +39,7 @@ RSpec.describe InternalId do
     end
   end
 
+  shared_examples_for 'a monotonically increasing id generator' do
     describe '.generate_next' do
       subject { described_class.generate_next(id_subject, scope, usage, init) }
 
@@ -66,19 +67,6 @@ RSpec.describe InternalId do
             expect(subject).to eq(project.issues.size + 1)
           end
         end
-
-      context 'with concurrent inserts on table' do
-        it 'looks up the record if it was created concurrently' do
-          args = { **scope, usage: described_class.usages[usage.to_s] }
-          record = double
-          expect(described_class).to receive(:find_by).with(args).and_return(nil)    # first call, record not present
-          expect(described_class).to receive(:find_by).with(args).and_return(record) # second call, record was created by another process
-          expect(described_class).to receive(:create!).and_raise(ActiveRecord::RecordNotUnique, 'record not unique')
-          expect(record).to receive(:increment_and_save!)
-
-          subject
-        end
-      end
       end
 
       it 'generates a strictly monotone, gapless sequence' do
@@ -102,7 +90,7 @@ RSpec.describe InternalId do
         it 'increments counter with in_transaction: "false"' do
           allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
 
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original
 
           subject
@@ -111,7 +99,7 @@ RSpec.describe InternalId do
 
       context 'when executed within transaction' do
         it 'increments counter with in_transaction: "true"' do
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original
 
           InternalId.transaction { subject }
@@ -161,7 +149,7 @@ RSpec.describe InternalId do
         it 'increments counter with in_transaction: "false"' do
           allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
 
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original
 
           subject
@@ -172,7 +160,7 @@ RSpec.describe InternalId do
         let(:value) { 2 }
 
         it 'increments counter with in_transaction: "true"' do
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original
 
           InternalId.transaction { subject }
@@ -232,7 +220,7 @@ RSpec.describe InternalId do
         it 'increments counter with in_transaction: "false"' do
           allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
 
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original
 
           subject
@@ -241,13 +229,26 @@ RSpec.describe InternalId do
 
       context 'when executed within transaction' do
         it 'increments counter with in_transaction: "true"' do
-        expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
+          expect(InternalId.internal_id_transactions_total).to receive(:increment)
             .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original
 
           InternalId.transaction { subject }
         end
       end
     end
+  end
+
+  context 'when the feature flag is disabled' do
+    stub_feature_flags(generate_iids_without_explicit_locking: false)
+
+    it_behaves_like 'a monotonically increasing id generator'
+  end
+
+  context 'when the feature flag is enabled' do
+    stub_feature_flags(generate_iids_without_explicit_locking: true)
+
+    it_behaves_like 'a monotonically increasing id generator'
+  end
 
   describe '#increment_and_save!' do
     let(:id) { create(:internal_id) }

spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb

@@ -165,9 +165,9 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
           3.times { supply.next_value }
         end
 
-        current_value = described_class.public_send(method_name, scope_value, &:current_value)
-
-        expect(current_value).to eq(iid + 3)
+        described_class.public_send(method_name, scope_value) do |supply|
+          expect(supply.next_value).to eq(iid + 4)
+        end
       end
     end