Use session-level lock timeout for migrations with disable_ddl_transaction!

CHANGELOG.md

@@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+## [2.0.0] - TBD
+### Changed
+- **BREAKING CHANGE**: Migrations using `disable_ddl_transaction!` now use session-level lock timeout (`SET lock_timeout`) instead of being skipped entirely. Previously, these migrations had no lock timeout applied. Now they will fail if they cannot acquire locks within the configured timeout period. This provides lock timeout protection for concurrent index creation and other non-transactional operations. The timeout is automatically reset after the migration completes.
+
+  **Migration Guide**: If you have migrations using `disable_ddl_transaction!` that previously worked but may take longer than your configured timeout to acquire locks, you should either:
+  - Use `disable_lock_timeout!` to explicitly disable the timeout for that migration
+  - Use `set_lock_timeout <seconds>` to set a longer timeout for that specific migration
+  - Adjust your default timeout configuration to accommodate these operations
+
 ## [1.5.0]
 ### Removed
 - Dropped support for Rails 6.0 and earlier

README.md

@@ -70,7 +70,8 @@ set a timeout for a particular migration.
 
 ## disable_ddl_transaction!
 
-If you use `disable_ddl_transaction!`, no lock timeout will occur
+When you use `disable_ddl_transaction!`, the gem automatically switches to using a session-level lock timeout instead of a transaction-level timeout. This is necessary because non-transactional migrations don't run inside a database transaction, so the `SET LOCAL` command (which only applies within a transaction) wouldn't work.
+
 ```ruby
   class AddMonkey < ActiveRecord::Migration
 
@@ -84,6 +85,46 @@ If you use `disable_ddl_transaction!`, no lock timeout will occur
   end
 ```
 
+For this migration, the gem will execute:
+```psql
+SET lock_timeout = '5s';  -- Session-level timeout
+-- Your migration code runs here
+RESET lock_timeout;       -- Reset to default after migration
+```
+
+This is particularly useful for operations that require `disable_ddl_transaction!`, such as:
+- Creating indexes concurrently (`add_index :table, :column, algorithm: :concurrently`)
+- Adding columns with a default value in older PostgreSQL versions
+- Other operations that cannot run inside a transaction
+
+**Note:** The lock timeout is automatically reset after the migration completes to avoid affecting subsequent database operations.
+
+**Important:** If you need to disable the lock timeout for a specific non-transactional migration (for example, if the operation legitimately needs to wait longer for locks), you can combine `disable_ddl_transaction!` with `disable_lock_timeout!`:
+
+```ruby
+  class AddIndexConcurrently < ActiveRecord::Migration
+    disable_ddl_transaction!
+    disable_lock_timeout!  # Explicitly disable timeout for this migration
+
+    def change
+      add_index :large_table, :column, algorithm: :concurrently
+    end
+  end
+```
+
+Alternatively, you can set a custom timeout for the migration:
+
+```ruby
+  class AddIndexConcurrently < ActiveRecord::Migration
+    disable_ddl_transaction!
+    set_lock_timeout 30  # Wait up to 30 seconds for locks
+
+    def change
+      add_index :large_table, :column, algorithm: :concurrently
+    end
+  end
+```
+
 ## Running the specs
 
 To run the specs you must have [PostgreSQL](https://www.postgresql.org/)

lib/migration_lock_timeout/lock_manager.rb

@@ -5,12 +5,27 @@ def migrate(direction)
       timeout_disabled = self.class.disable_lock_timeout
       time = self.class.lock_timeout_override ||
         MigrationLockTimeout.try(:config).try(:default_timeout)
-      if !timeout_disabled && direction == :up && time && !disable_ddl_transaction
+
+      if !timeout_disabled && direction == :up && time
         safety_assured? do
-          execute "SET LOCAL lock_timeout = '#{time}s'"
+          if disable_ddl_transaction
+            # Use session-level timeout for non-transactional migrations
+            execute "SET lock_timeout = '#{time}s'"
+          else
+            # Use transaction-level timeout for transactional migrations
+            execute "SET LOCAL lock_timeout = '#{time}s'"
+          end
         end
       end
+
       super
+    ensure
+      # Reset session-level timeout after non-transactional migrations
+      if !timeout_disabled && direction == :up && time && disable_ddl_transaction
+        safety_assured? do
+          execute "RESET lock_timeout"
+        end
+      end
     end
 
     def safety_assured?

spec/migration_lock_timeout/migration_spec.rb

@@ -176,19 +176,24 @@ def down
         end
       end
 
-      it 'runs migrate up without timeout' do
+      it 'runs migrate up with session-level timeout' do
         migration = AddMonkey.new
         expect_create_table
-        expect(ActiveRecord::Base.connection).not_to receive(:execute).
-          with("SET LOCAL lock_timeout = '5s'").
+        expect(ActiveRecord::Base.connection).to receive(:execute).
+          with("SET lock_timeout = '5s'").
+          and_call_original
+        expect(ActiveRecord::Base.connection).to receive(:execute).
+          with("RESET lock_timeout").
           and_call_original
         migration.migrate(:up)
       end
 
       it 'runs migrate down without timeout' do
         migration = AddMonkey.new
         expect(ActiveRecord::Base.connection).not_to receive(:execute).
-          with("SET LOCAL lock_timeout = '5s'")
+          with("SET lock_timeout = '5s'")
+        expect(ActiveRecord::Base.connection).not_to receive(:execute).
+          with("RESET lock_timeout")
         migration.migrate(:down)
       end
     end