Use session-level lock timeout for migrations with disable_ddl_transaction!

[pjmartorell]

Summary

This PR addresses issue #16 by implementing session-level lock timeouts for migrations that use disable_ddl_transaction!.

Problem

Previously, migrations using disable_ddl_transaction! had no lock timeout protection because the gem skipped timeout application entirely. This left concurrent index creation and other non-transactional operations vulnerable to indefinite lock waits, which could cause production issues (as mentioned in the issue comments).

The original implementation couldn't use SET LOCAL lock_timeout because it only works within transactions, and disable_ddl_transaction! runs migrations outside of transactions.

Solution

• Non-transactional migrations: Now use session-level timeout (SET lock_timeout = '5s')
• Transactional migrations: Continue using transaction-level timeout (SET LOCAL lock_timeout = '5s')
• Automatic cleanup: Session-level timeout is automatically reset after non-transactional migrations complete (RESET lock_timeout)

Breaking Change ⚠️

This is a breaking change that bumps the version to 2.0.0:

Before: Migrations with disable_ddl_transaction! had no timeout → waited indefinitely
After: These migrations now fail if locks can't be acquired within the configured timeout

Migration Guide

Users can adapt in three ways:

1. Disable timeout for specific migration:

class AddIndexConcurrently < ActiveRecord::Migration
  disable_ddl_transaction!
  disable_lock_timeout!  # Explicitly opt-out
  
  def change
    add_index :large_table, :column, algorithm: :concurrently
  end
end

2. Set custom timeout:

class AddIndexConcurrently < ActiveRecord::Migration
  disable_ddl_transaction!
  set_lock_timeout 30  # Wait up to 30 seconds
  
  def change
    add_index :large_table, :column, algorithm: :concurrently
  end
end

3. Adjust default configuration in the initializer

Changes

• Modified LockManager#migrate to detect disable_ddl_transaction and use appropriate timeout type
• Added ensure block to reset session-level timeout after non-transactional migrations
• Updated tests to verify session-level timeout behavior (all tests passing ✅)
• Comprehensive documentation updates in README and CHANGELOG
• Added migration examples and troubleshooting guide

Testing

All tests pass on ActiveRecord 7.1 and 7.1 with strong_migrations:

13 examples, 0 failures

Documentation

• Updated README with technical explanation of why session-level timeouts are needed
• Added code examples for both opting out and customizing timeouts
• Added CHANGELOG entry with breaking change notice and migration guide
• Marked as version 2.0.0 per semantic versioning

Fixes #16

[Copilot]

Pull Request Overview

This PR implements session-level lock timeouts for migrations using disable_ddl_transaction!, addressing a gap where these migrations previously had no lock timeout protection. The change ensures concurrent index creation and other non-transactional operations fail fast rather than waiting indefinitely for locks.

Key Changes:

• Modified LockManager#migrate to apply session-level timeouts (SET lock_timeout) for non-transactional migrations
• Added automatic cleanup via ensure block to reset session-level timeout after migration
• Updated tests to verify new session-level timeout behavior

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/migration_lock_timeout/lock_manager.rb	Implements session-level timeout logic with conditional execution based on disable_ddl_transaction flag
spec/migration_lock_timeout/migration_spec.rb	Updates test expectations to verify session-level timeout setting and reset behavior
README.md	Documents the new session-level timeout behavior and provides examples for opting out or customizing timeouts
CHANGELOG.md	Documents breaking change with migration guide for version 2.0.0

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The time variable is interpolated directly into the SQL command without sanitization. While this appears to be controlled by configuration, consider using parameterized queries or explicitly validating that time is numeric to prevent potential SQL injection if configuration sources become compromised.

Suggested change

	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'"
	# Validate that time is numeric to prevent SQL injection
	begin
	numeric_time = Integer(time)
	rescue ArgumentError, TypeError
	raise "lock_timeout value must be an integer"
	end
	safety_assured? do
	if disable_ddl_transaction
	# Use session-level timeout for non-transactional migrations
	execute "SET lock_timeout = '#{numeric_time}s'"
	else
	# Use transaction-level timeout for transactional migrations
	execute "SET LOCAL lock_timeout = '#{numeric_time}s'"