[#73613] Improve peformance of bulk semantic ID allocation

app/models/projects/semantic_identifier.rb

@@ -36,16 +36,8 @@ module Projects::SemanticIdentifier
   # Uses an advisory lock scoped to this project to serialize concurrent allocations
   # without blocking unrelated project row writes.
   def allocate_wp_semantic_identifier!
-    seq = OpenProject::Mutex.with_advisory_lock(
-      self.class,
-      "wp_sequence_#{id}"
-    ) do
-      self.class.connection.select_value(<<~SQL.squish)
-        UPDATE projects
-        SET wp_sequence_counter = wp_sequence_counter + 1
-        WHERE id = #{self.class.connection.quote(id)}
-        RETURNING wp_sequence_counter
-      SQL
+    seq = OpenProject::Mutex.with_advisory_lock(self.class, "wp_sequence_#{id}") do
+      allocate_sequence_range!.first
     end
 
     [seq, "#{identifier}-#{seq}"]
@@ -60,14 +52,35 @@ def previous_semantic_identifier
     return nil if candidates.empty?
 
     taken = self.class
-      .where.not(id:)
-      .where("LOWER(identifier) IN (?)", candidates.map(&:downcase))
-      .pluck(:identifier)
-      .to_set(&:downcase)
+                .where.not(id:)
+                .where("LOWER(identifier) IN (?)", candidates.map(&:downcase))
+                .pluck(:identifier)
+                .to_set(&:downcase)
 
     candidates.find { |slug| taken.exclude?(slug.downcase) }
   end
 
+  # Atomically reserves `work_package_ids.size` consecutive sequence numbers,
+  # bulk-updates those work packages' sequence_number and identifier columns,
+  # and (by default) inserts alias rows for every historical slug prefix of
+  # this project.
+  #
+  # Pass insert_aliases: false when the caller will run seed_alias_table
+  # immediately after (e.g. the conversion backfill path), to avoid
+  # duplicating the alias insertion work.
+  def reserve_semantic_id_block!(work_package_ids, insert_aliases: true)
+    count = work_package_ids.size
+    return if count.zero?
+
+    range = allocate_sequence_range!(count)
+    sorted_ids = work_package_ids.sort
+
+    WorkPackageSemanticAlias.transaction do
+      bulk_assign_sequence_numbers!(sorted_ids, range)
+      insert_sequence_aliases!(sorted_ids, range) if insert_aliases
+    end
+  end
+
   # Called after this project's identifier is renamed. Atomically:
   # 1. Appends new-prefix aliases for every WP that ever carried an old-prefix alias.
   # 2. Updates identifier on resident WPs to the new prefix.
@@ -91,31 +104,66 @@ def previous_semantic_identifier_candidates
       .select { |slug| ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers.valid_format?(slug) }
   end
 
+  def bulk_assign_sequence_numbers!(sorted_ids, range)
+    proj_ident = self.class.connection.quote(identifier)
+    values = sorted_ids.zip(range)
+                       .map { |wp_id, seq| "(#{wp_id}, #{seq})" }
+                       .join(", ")
+    self.class.connection.execute(<<~SQL.squish)
+      UPDATE work_packages
+      SET sequence_number = v.seq,
+          identifier      = #{proj_ident} || '-' || v.seq::text
+      FROM (VALUES #{values}) AS v(id, seq)
+      WHERE work_packages.id = v.id
+    SQL
+  end
+
+  def insert_sequence_aliases!(sorted_ids, range)
+    slug_prefixes = slugs.pluck(:slug)
+    alias_rows = sorted_ids.zip(range).flat_map do |wp_id, seq|
+      slug_prefixes.map { |pfx| { identifier: "#{pfx}-#{seq}", work_package_id: wp_id } }
+    end
+    WorkPackageSemanticAlias.insert_all(alias_rows, unique_by: :identifier) if alias_rows.any?
+  end
+
+  # Atomically reserves `count` sequence numbers and returns them as a Range.
+  # The UPDATE is atomic at the PostgreSQL row level, so concurrent callers
+  # serialize without a separate advisory lock.
+  def allocate_sequence_range!(count = 1)
+    base = self.class.connection.select_value(<<~SQL.squish) - count
+      UPDATE projects
+      SET wp_sequence_counter = wp_sequence_counter + #{count}
+      WHERE id = #{id}
+      RETURNING wp_sequence_counter
+    SQL
+    (base + 1)..(base + count)
+  end
+
   # For every alias row whose identifier starts with the old prefix, inserts a
   # corresponding row with the new prefix. This covers WPs still in the project
   # as well as any that have moved out but still carry old-prefix alias rows.
   def append_aliases_with_new_prefix(like_pattern:, prefix:, new_prefix:, batch_size:)
     WorkPackageSemanticAlias
       .where("identifier LIKE ?", like_pattern)
       .in_batches(of: batch_size) do |relation|
-        now = Time.current
-        WorkPackageSemanticAlias.connection.execute(
-          WorkPackageSemanticAlias.sanitize_sql([<<~SQL.squish, { prefix:, new_prefix:, now: }])
+      now = Time.current
+      WorkPackageSemanticAlias.connection.execute(
+        WorkPackageSemanticAlias.sanitize_sql([<<~SQL.squish, { prefix:, new_prefix:, now: }])
             INSERT INTO work_package_semantic_aliases (identifier, work_package_id, created_at, updated_at)
             SELECT REPLACE(identifier, :prefix, :new_prefix), work_package_id, :now, :now
             FROM (#{relation.to_sql}) AS batch
             ON CONFLICT (identifier) DO NOTHING
           SQL
-        )
-      end
+      )
+    end
   end
 
   # Updates the identifier column on all resident WPs to replace the old prefix with the new one.
   def rewrite_semantic_ids(like_pattern:, prefix:, new_prefix:, batch_size:)
     WorkPackage
       .where("identifier LIKE ?", like_pattern)
       .in_batches(of: batch_size) do |relation|
-        relation.update_all(["identifier = REPLACE(identifier, ?, ?)", prefix, new_prefix])
-      end
+      relation.update_all(["identifier = REPLACE(identifier, ?, ?)", prefix, new_prefix])
+    end
   end
 end

app/services/project_identifiers/convert_project_to_semantic_service.rb

@@ -97,9 +97,10 @@ def reset_stale_identifiers
     end
 
     def backfill_missing_ids
-      WorkPackage.where(project:).unsequenced.find_each do |wp|
-        seq, identifier = project.allocate_wp_semantic_identifier!
-        wp.update_columns(sequence_number: seq, identifier:)
+      WorkPackage.where(project:)
+                 .unsequenced
+                 .in_batches(order: :asc) do |batch|
+        project.reserve_semantic_id_block!(batch.pluck(:id), insert_aliases: false)
       end
     end
 

app/services/work_packages/update_service.rb

@@ -110,7 +110,9 @@ def cleanup(work_package)
   end
 
   def update_semantic_ids(work_packages)
-    work_packages.each(&:allocate_and_register_semantic_id)
+    return if work_packages.empty?
+
+    work_packages.first.project.reserve_semantic_id_block!(work_packages.map(&:id))
   end
 
   def delete_relations(work_packages)

spec/models/projects/semantic_identifier_spec.rb

@@ -59,6 +59,90 @@
     end
   end
 
+  describe "#reserve_semantic_id_block!" do
+    let(:project) { create(:project, identifier: "PROJ") }
+    let(:wp1) { create(:work_package, project:) }
+    let(:wp2) { create(:work_package, project:) }
+    let(:wp3) { create(:work_package, project:) }
+
+    # WPs created in semantic mode already have allocated IDs; reset to a
+    # clean slate so reserve_semantic_id_block! can be tested in isolation.
+    def reset_wps(*wps)
+      ids = wps.map(&:id)
+      WorkPackage.where(id: ids).update_all(sequence_number: nil, identifier: nil)
+      WorkPackageSemanticAlias.where(work_package_id: ids).delete_all
+      project.update_columns(wp_sequence_counter: 0)
+    end
+
+    context "with an empty list" do
+      it "is a no-op" do
+        before_count = project.reload.wp_sequence_counter
+        project.reserve_semantic_id_block!([])
+        expect(project.reload.wp_sequence_counter).to eq(before_count)
+      end
+    end
+
+    context "with work package ids" do
+      before { reset_wps(wp1, wp2, wp3) }
+
+      it "advances the counter by the number of ids" do
+        project.reserve_semantic_id_block!([wp1.id, wp2.id, wp3.id])
+        expect(project.reload.wp_sequence_counter).to eq(3)
+      end
+
+      it "allocates all sequence numbers in a single call to allocate_sequence_range!" do
+        allow(project).to receive(:allocate_sequence_range!).and_call_original
+        project.reserve_semantic_id_block!([wp1.id, wp2.id, wp3.id])
+        expect(project).to have_received(:allocate_sequence_range!).with(3).once
+      end
+
+      it "assigns consecutive sequence numbers" do
+        project.reserve_semantic_id_block!([wp1.id, wp2.id, wp3.id])
+        expect(wp1.reload.sequence_number).to eq(1)
+        expect(wp2.reload.sequence_number).to eq(2)
+        expect(wp3.reload.sequence_number).to eq(3)
+      end
+
+      it "sets identifiers to the project-prefix form" do
+        project.reserve_semantic_id_block!([wp1.id, wp2.id, wp3.id])
+        expect(wp1.reload.identifier).to eq("PROJ-1")
+        expect(wp2.reload.identifier).to eq("PROJ-2")
+        expect(wp3.reload.identifier).to eq("PROJ-3")
+      end
+
+      it "continues from the existing counter value" do
+        project.update_columns(wp_sequence_counter: 10)
+        project.reserve_semantic_id_block!([wp1.id, wp2.id])
+        expect(wp1.reload.sequence_number).to eq(11)
+        expect(wp2.reload.sequence_number).to eq(12)
+      end
+
+      context "when insert_aliases: true (default)" do
+        it "creates alias rows for each slug prefix" do
+          project.reserve_semantic_id_block!([wp1.id, wp2.id])
+          expect(WorkPackageSemanticAlias.where(work_package: wp1).pluck(:identifier))
+            .to contain_exactly("PROJ-1")
+          expect(WorkPackageSemanticAlias.where(work_package: wp2).pluck(:identifier))
+            .to contain_exactly("PROJ-2")
+        end
+
+        it "creates alias rows for all historical slug prefixes" do
+          FriendlyId::Slug.create!(sluggable: project, slug: "OLDPROJ")
+          project.reserve_semantic_id_block!([wp1.id])
+          expect(WorkPackageSemanticAlias.where(work_package: wp1).pluck(:identifier))
+            .to contain_exactly("PROJ-1", "OLDPROJ-1")
+        end
+      end
+
+      context "when insert_aliases: false" do
+        it "skips alias insertion" do
+          project.reserve_semantic_id_block!([wp1.id, wp2.id], insert_aliases: false)
+          expect(WorkPackageSemanticAlias.where(work_package: [wp1, wp2])).not_to exist
+        end
+      end
+    end
+  end
+
   describe "#handle_semantic_rename" do
     let(:project) { create(:project, identifier: "PROJ", wp_sequence_counter: 0) }
     let(:target_project) { create(:project, identifier: "OTHER", wp_sequence_counter: 0) }

spec/services/work_packages/semantic_ids/integration_spec.rb

@@ -109,6 +109,16 @@
       expect(work_package.reload.identifier).to start_with("DEST-")
     end
 
+    it "allocates sequence numbers in a single batch when moving a WP with descendants" do
+      children = create_list(:work_package, 4, project:, parent: work_package)
+
+      WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project)
+
+      moved = [work_package, *children].map { |wp| wp.reload.identifier }
+      expect(moved).to all(start_with("DEST-"))
+      expect(moved.map { |id| id.split("-").last.to_i }).to match_array(1..5)
+    end
+
     it "old identifier still resolves to the WP" do
       WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project)
       expect(WorkPackage.find_by_display_id("PROJ-5")).to eq(work_package)