Add helper to override timeout for concurrent statements

Statements like `CRETE INDEX ... CONCURRENTLY` might take longer than
the configured `max_migration_statement_runtime_in_seconds` value.
With the helper function `VCAP::Migration.with_concurrent_timeout(self)`
any following statement will use the configured `migration_psql_concurrent_statement_timeout_in_seconds` value instead.

Example usage:
```
Sequel.migration do
        up do
          VCAP::Migration.with_concurrent_timeout(self) do
            # slow migration
          end
        end
```

Author: johha

config/cloud_controller.yml

@@ -329,6 +329,7 @@ max_annotations_per_resource: 200
 max_labels_per_resource: 50
 max_migration_duration_in_minutes: 45
 max_migration_statement_runtime_in_seconds: 30
+migration_psql_concurrent_statement_timeout_in_seconds: 1800
 db:
   log_level: 'debug'
   ssl_verify_hostname: false

db/migrations/20231113105256_add_service_plan_id_index.rb

@@ -2,10 +2,14 @@
   no_transaction # to use the 'concurrently' option
 
   up do
-    add_index :service_plan_visibilities, :service_plan_id, name: :spv_service_plan_id_index, concurrently: true if database_type == :postgres
+    VCAP::Migration.with_concurrent_timeout(self) do
+      add_index :service_plan_visibilities, :service_plan_id, name: :spv_service_plan_id_index, concurrently: true if database_type == :postgres
+    end
   end
 
   down do
-    drop_index :service_plan_visibilities, nil, name: :spv_service_plan_id_index, concurrently: true if database_type == :postgres
+    VCAP::Migration.with_concurrent_timeout(self) do
+      drop_index :service_plan_visibilities, nil, name: :spv_service_plan_id_index, concurrently: true if database_type == :postgres
+    end
   end
 end

db/migrations/20240219113000_add_routes_space_id_index.rb

@@ -2,10 +2,14 @@
   no_transaction # to use the 'concurrently' option
 
   up do
-    add_index :routes, :space_id, name: :routes_space_id_index, if_not_exists: true, concurrently: true if database_type == :postgres
+    VCAP::Migration.with_concurrent_timeout(self) do
+      add_index :routes, :space_id, name: :routes_space_id_index, if_not_exists: true, concurrently: true if database_type == :postgres
+    end
   end
 
   down do
-    drop_index :routes, :space_id, name: :routes_space_id_index, if_exists: true, concurrently: true if database_type == :postgres
+    VCAP::Migration.with_concurrent_timeout(self) do
+      drop_index :routes, :space_id, name: :routes_space_id_index, if_exists: true, concurrently: true if database_type == :postgres
+    end
   end
 end

db/migrations/20240222131500_change_delayed_jobs_reserve_index.rb

@@ -3,17 +3,21 @@
 
   up do
     if database_type == :postgres
-      drop_index :delayed_jobs, nil, name: :delayed_jobs_reserve, if_exists: true, concurrently: true
-      add_index :delayed_jobs, %i[queue locked_at locked_by failed_at run_at priority],
-                where: { failed_at: nil }, name: :delayed_jobs_reserve, if_not_exists: true, concurrently: true
+      VCAP::Migration.with_concurrent_timeout(self) do
+        drop_index :delayed_jobs, nil, name: :delayed_jobs_reserve, if_exists: true, concurrently: true
+        add_index :delayed_jobs, %i[queue locked_at locked_by failed_at run_at priority],
+                  where: { failed_at: nil }, name: :delayed_jobs_reserve, if_not_exists: true, concurrently: true
+      end
     end
   end
 
   down do
     if database_type == :postgres
-      drop_index :delayed_jobs, nil, name: :delayed_jobs_reserve, if_exists: true, concurrently: true
-      add_index :delayed_jobs, %i[queue locked_at locked_by failed_at run_at priority],
-                name: :delayed_jobs_reserve, if_not_exists: true, concurrently: true
+      VCAP::Migration.with_concurrent_timeout(self) do
+        drop_index :delayed_jobs, nil, name: :delayed_jobs_reserve, if_exists: true, concurrently: true
+        add_index :delayed_jobs, %i[queue locked_at locked_by failed_at run_at priority],
+                  name: :delayed_jobs_reserve, if_not_exists: true, concurrently: true
+      end
     end
   end
 end

db/migrations/20240314131908_add_user_guid_to_jobs_table.rb

@@ -4,9 +4,12 @@
 
   up do
     if database_type == :postgres
+      db = self
       alter_table :jobs do
         add_column :user_guid, String, size: 255, if_not_exists: true
-        add_index :user_guid, name: :jobs_user_guid_index, if_not_exists: true, concurrently: true
+        VCAP::Migration.with_concurrent_timeout(db) do
+          add_index :user_guid, name: :jobs_user_guid_index, if_not_exists: true, concurrently: true
+        end
       end
 
     elsif database_type == :mysql
@@ -21,8 +24,11 @@
 
   down do
     if database_type == :postgres
+      db = self
       alter_table :jobs do
-        drop_index  :user_guid, name: :jobs_user_guid_index, if_exists: true, concurrently: true
+        VCAP::Migration.with_concurrent_timeout(db) do
+          drop_index :user_guid, name: :jobs_user_guid_index, if_exists: true, concurrently: true
+        end
         drop_column :user_guid, if_exists: true
       end
     end

lib/cloud_controller/config_schemas/base/api_schema.rb

@@ -87,6 +87,7 @@ class ApiSchema < VCAP::Config
 
             optional(:max_migration_duration_in_minutes) => Integer,
             optional(:max_migration_statement_runtime_in_seconds) => Integer,
+            optional(:migration_psql_concurrent_statement_timeout_in_seconds) => Integer,
             optional(:migration_psql_worker_memory_kb) => Integer,
             db: {
               optional(:database) => Hash, # db connection hash for sequel

lib/cloud_controller/config_schemas/base/migrate_schema.rb

@@ -8,6 +8,7 @@ class MigrateSchema < VCAP::Config
           {
             optional(:max_migration_duration_in_minutes) => Integer,
             optional(:max_migration_statement_runtime_in_seconds) => Integer,
+            optional(:migration_psql_concurrent_statement_timeout_in_seconds) => Integer,
             optional(:migration_psql_worker_memory_kb) => Integer,
 
             db: {

lib/cloud_controller/db.rb

@@ -148,6 +148,8 @@ def empty_from_sql
 # the migration methods.
 module VCAP
   module Migration
+    PSQL_DEFAULT_STATEMENT_TIMEOUT = 30_000
+
     def self.timestamps(migration, table_key)
       created_at_idx = :"#{table_key}_created_at_index" if table_key
       updated_at_idx = :"#{table_key}_updated_at_index" if table_key
@@ -219,6 +221,20 @@ def self.uuid_function(migration)
       end
     end
 
+    # Concurrent migrations can take a long time to run, so this helper can be used to override 'max_migration_statement_runtime_in_seconds' for a specific migration.
+    # REF: https://www.postgresql.org/docs/current/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY
+    def self.with_concurrent_timeout(db, &block)
+      concurrent_timeout_seconds = VCAP::CloudController::Config.config&.get(:migration_psql_concurrent_statement_timeout_in_seconds) || PSQL_DEFAULT_STATEMENT_TIMEOUT
+
+      if concurrent_timeout_seconds && db.database_type == :postgres
+        original_timeout = db.fetch("select setting from pg_settings where name = 'statement_timeout'").first[:setting]
+        db.run("SET statement_timeout TO #{concurrent_timeout_seconds * 1000}")
+      end
+      block.call
+    ensure
+      db.run("SET statement_timeout TO #{original_timeout}") if original_timeout && db.database_type == :postgres
+    end
+
     def self.logger
       Steno.logger('cc.db.migrations')
     end

lib/cloud_controller/db_migrator.rb

@@ -15,7 +15,7 @@ def initialize(db, max_migration_duration_in_minutes=nil, max_migration_statemen
     @timeout_in_minutes = default_two_weeks(max_migration_duration_in_minutes)
 
     @max_statement_runtime_in_milliseconds = if max_migration_statement_runtime_in_seconds.nil? || max_migration_statement_runtime_in_seconds <= 0
-                                               30_000
+                                               VCAP::Migration::PSQL_DEFAULT_STATEMENT_TIMEOUT
                                              else
                                                max_migration_statement_runtime_in_seconds * 1000
                                              end

spec/migrations/Readme.md

@@ -68,7 +68,10 @@ To create resilient and reliable migrations, follow these guidelines:
    ```
 1. If you're writing a uniqueness constraint where some of the values can be null, remember that `null != null`. For instance, the values `[1, 1, null]` and `[1, 1, null]` are considered unique. Uniqueness constraints only work on columns that do not allow `NULL` as a value. If this is the case, change the column to disallow `NULL` and set the default to an empty string instead.
 1. If you need to execute different operations for MySQL and Postgres, you can check the database type as follows: `... if database_type == :postgres` or `... if database_type == :mysql`. If the differences are too big, consider writing separate migrations for each database type.
-1. Be sure that with real world table sizes and load each sql query will finish in reasonable time inside your migration. **There is a hard limit of 30s in place to protect against outages caused by long-running migrations**. If you need to run a long-running migration, consider breaking it up into smaller parts. If you make use of table locking, be sure to run any query with real world table sizes sub 2 seconds to not cause issues due to table locks and waiting queries. In case a single statement exceeds 30s(default), the migration is aborted. An operator can overwrite this behaviour by setting the `max_migration_statement_runtime_in_seconds` config property.
+1. Be sure that with real world table sizes and load each sql query will finish in reasonable time inside your migration. **There is a hard limit of 30s in place to protect against outages caused by long-running migrations**.
+   If you need to run a long-running migration, consider breaking it up into smaller parts. If you make use of table locking, be sure to run any query with real world table sizes sub 2 seconds to not cause issues due to table locks and waiting queries.
+   In case a single statement exceeds 30s(default), the migration is aborted. An operator can overwrite this behaviour by setting the `max_migration_statement_runtime_in_seconds` config property.  
+   Since index creation can take a long time, especially when using `CONCURRENTLY`, an operator can set the `migration_psql_concurrent_statement_timeout` config property to a higher value to allow for longer running statements.
 
 # Sequel Migration Tests