Fix async index creation for long tables (!121513) · Merge requests · GitLab.org / GitLab

What does this MR do and why?

This MR fixes the async index creation when adding index for a partitioned table with a long name. The old validation allows 63 characters, however for partitioned tables, the schema name is also prepended.

The new validation rule should be (max length is 127): schema_name(63) + .(1) + table_name(63)

Note: I added simple length validation, I suppose at some point we might want to validate the format of the table_name (no special characters, etc.).

Database

The new validation works with all existing records, used PG.ai:

Gitlab::Database::AsyncIndexes::PostgresAsyncIndex.all.all?(&:valid?)

Migration

Up:

main: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: migrating ========
main: -- transaction_open?()
main:    -> 0.0000s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- execute("ALTER TABLE postgres_async_indexes\nADD CONSTRAINT check_schema_and_name_length\nCHECK ( char_length(table_name) <= 127 )\nNOT VALID;\n")
main:    -> 0.0009s
main: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: migrated (0.0674s)

main: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: migrating ======
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0003s
main: -- execute("ALTER TABLE postgres_async_indexes VALIDATE CONSTRAINT check_schema_and_name_length;")
main:    -> 0.0005s
main: -- execute("RESET statement_timeout")
main:    -> 0.0004s
main: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: migrated (0.0047s)

main: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: migrating =====
main: -- transaction_open?()
main:    -> 0.0000s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- execute("            ALTER TABLE postgres_async_indexes\n            DROP CONSTRAINT IF EXISTS check_e64ff4359e\n")
main:    -> 0.0007s
main: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: migrated (0.0042s)


ci: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: migrating ========
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- execute("ALTER TABLE postgres_async_indexes\nADD CONSTRAINT check_schema_and_name_length\nCHECK ( char_length(table_name) <= 127 )\nNOT VALID;\n")
ci:    -> 0.0009s
ci: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: migrated (0.0118s)

ci: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: migrating ======
ci: -- execute("SET statement_timeout TO 0")
ci:    -> 0.0005s
ci: -- execute("ALTER TABLE postgres_async_indexes VALIDATE CONSTRAINT check_schema_and_name_length;")
ci:    -> 0.0005s
ci: -- execute("RESET statement_timeout")
ci:    -> 0.0003s
ci: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: migrated (0.0097s)

ci: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: migrating =====
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- execute("            ALTER TABLE postgres_async_indexes\n            DROP CONSTRAINT IF EXISTS check_e64ff4359e\n")
ci:    -> 0.0006s
ci: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: migrated (0.0098s)

Down:

ci: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: reverting =====
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- execute("ALTER TABLE postgres_async_indexes\nADD CONSTRAINT check_e64ff4359e\nCHECK ( char_length(table_name) <= 63 )\nNOT VALID;\n")
ci:    -> 0.0008s
ci: -- execute("SET statement_timeout TO 0")
ci:    -> 0.0003s
ci: -- execute("ALTER TABLE postgres_async_indexes VALIDATE CONSTRAINT check_e64ff4359e;")
ci:    -> 0.0005s
ci: -- execute("RESET statement_timeout")
ci:    -> 0.0003s
ci: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: reverted (0.3117s)

main: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: reverting =====
main: -- transaction_open?()
main:    -> 0.0000s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- execute("ALTER TABLE postgres_async_indexes\nADD CONSTRAINT check_e64ff4359e\nCHECK ( char_length(table_name) <= 63 )\nNOT VALID;\n")
main:    -> 0.0020s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0004s
main: -- execute("ALTER TABLE postgres_async_indexes VALIDATE CONSTRAINT check_e64ff4359e;")
main:    -> 0.0006s
main: -- execute("RESET statement_timeout")
main:    -> 0.0003s
main: == 20230523074517 RemoveOldAsyncIndexTableNameLengthConstraint: reverted (0.3975s)


ci: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: reverting ======
ci: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: reverted (0.0087s)

main: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: reverting ======
main: == 20230523074248 ValidateAsyncIndexTableNameLengthConstraint: reverted (0.0021s)

ci: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: reverting ========
ci: -- transaction_open?()
ci:    -> 0.0000s
ci: -- transaction_open?()
ci:    -> 0.0001s
ci: -- execute("            ALTER TABLE postgres_async_indexes\n            DROP CONSTRAINT IF EXISTS check_schema_and_name_length\n")
ci:    -> 0.0088s
ci: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: reverted (0.1067s)

main: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: reverting ========
main: -- transaction_open?()
main:    -> 0.0001s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- execute("            ALTER TABLE postgres_async_indexes\n            DROP CONSTRAINT IF EXISTS check_schema_and_name_length\n")
main:    -> 0.0010s
main: == 20230523073455 AddNewAsyncIndexTableNameLengthConstraint: reverted (0.0168s)

How to set up and validate locally

Create a migration.

class AddAsyncIndexToVsaIssues < Gitlab::Database::Migration[2.1]
  include Gitlab::Database::PartitioningMigrationHelpers

  TABLE_NAME = :analytics_cycle_analytics_issue_stage_events
  COLUMN_NAMES = %I[stage_event_hash_id group_id end_event_timestamp issue_id]
  INDEX_NAME = 'index_issue_stage_events_for_consistency_check'

  disable_ddl_transaction!

  def up
    # The table is hash partitioned
    each_partition(TABLE_NAME) do |partition, partition_index_name|
      prepare_async_index(
        partition.identifier,
        COLUMN_NAMES,
        name: partition_index_name
      )
    end
  end

  def down
    each_partition(TABLE_NAME) do |partition, partition_index_name|
      unprepare_async_index_by_name(partition.identifier, partition_index_name)
    end
  end

  private

  def each_partition(table_name)
    partitioned_table = find_partitioned_table(table_name)
    partitioned_table.postgres_partitions.order(:name).each do |partition|
      partition_index_name = generated_index_name(partition.identifier, INDEX_NAME)

      yield partition, partition_index_name
    end
  end
end

Run: rake db:migrate
Run in console: Gitlab::Database::AsyncIndexes.execute_pending_actions!(how_many: 300)
No error should be raised.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

I have evaluated the MR acceptance checklist for this MR.

Edited May 23, 2023 by Adam Hegyi

Fix async index creation for long tables

What does this MR do and why?

Database

Migration

How to set up and validate locally

MR acceptance checklist

Merge request reports