Pessimistic locking via add_lock! adds lock clause on nested selects and can cause deadlock

[metaskills]

Reported by Joe Rafaniello | June 12th, 2009 @ 05:28 PM

I am using the 2.2.16 version of the adapter and noticed that I occasionally receive deadlocks while using the :lock option, like this:

Account.find(:first,
  :conditions => [cond, *cond_param],
  :order => "priority, id",
  :lock => "WITH(UPDLOCK, ROWLOCK)"
)

The error shows itself as a process being chosen as a deadlock victim. Notice that we have nested selects and that each one has a WITH(UPDLOCK,ROWLOCK) and they are on the same table:

An error has occurred during work processing: DBI::DatabaseError: 37000 (1205) [unixODBC][FreeTDS][SQL Server]Transaction (Process ID 62) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.: SELECT TOP 1 * FROM [accounts] WITH(UPDLOCK, ROWLOCK) WHERE (state = 'active' and acct_type = 'priority' and (created_on is null or created_on <= '2009-06-04 23:06:27.404') and (task_id is NULL or task_id not in ( select task_id FROM accounts WITH(UPDLOCK, ROWLOCK) where state = 'expired' and task_id is not NULL )) and (region = 'NorthEast' or region = '' or region IS NULL) and (role in ('administrator','operator') or role = '' or role IS NULL) ORDER BY priority, id /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/connection_adapters/abstract_adapter.rb:188:in log' /var/www/miq/vmdb/vendor/gems/rails-sqlserver-2000-2005-adapter-2.2.16/lib/active_record/connection_adapters/sqlserver_adapter.rb:811:inraw_execute' /var/www/miq/vmdb/vendor/gems/rails-sqlserver-2000-2005-adapter-2.2.16/lib/active_record/connection_adapters/sqlserver_adapter.rb:834:in raw_select' /var/www/miq/vmdb/vendor/gems/rails-sqlserver-2000-2005-adapter-2.2.16/lib/active_record/connection_adapters/sqlserver_adapter.rb:787:inselect' /var/www/miq/vmdb/vendor/gems/rails-sqlserver-2000-2005-adapter-2.2.16/lib/active_record/connection_adapters/sqlserver_adapter.rb:787:in select' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:7:inselect_all_without_query_cache' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/connection_adapters/abstract/query_cache.rb:62:in select_all' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/base.rb:635:infind_by_sql' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/base.rb:1490:in find_every' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/base.rb:1452:infind_initial' /var/www/miq/vmdb/config/../vendor/gems/activerecord-2.2.2/lib/active_record/base.rb:587:in find'

I found that the sql.gsub! line in the add_lock! method of the adapter is adding the LOCK phrase to each of the FROM clauses. I have a hack to the adapter to only add the lock for the first select we find. I'm not sure this is really the correct way of handling it. I'm not sure how to handle the JOIN gsub! either. Any ideas?

Attached is a svn diff of my local patch.
http://rails-sqlserver.lighthouseapp.com/attachments/132867/add_lock_deadlock.patch

[metaskills]

Been busy, sorry for the delay. This looks good, and the change does not break any tests, but a regression test would have been nice. In fact... I'll need one. I can not get a test I tried writing for you to fail. For instance, I did this in pessimistic locking test.

Person.find(:first, 
  :conditions => "first_name IN (SELECT [first_name] FROM [people])", 
  :lock => "WITH(UPDLOCK, ROWLOCK)")

Which results in the following SQL that appears to work.

"SELECT TOP 1 * FROM [people] WITH(UPDLOCK, ROWLOCK) WHERE (first_name IN (SELECT [first_name] FROM [people] WITH(UPDLOCK, ROWLOCK))) "

I need more to go on. Can you help write a failing test?

[jrafanie]

Ken, I've been swamped too. I'll try to get back to this and provide you a failing test.

[jrafanie]

Ken, we have tracked down the deadlock to something unrelated to the reported issue namely two threads blocking each other: one holding a DB lock, the other executing a blocking operation which needs the row locked by the first thread.

With this said, I believe the gsub! is dangerous as I would expect the user would only want to lock the row for the original select and not every table included in the query.

I'm not sure how you want to resolve this.

[metaskills]

Thanks. You know what, I've had the same issue too! We have many databases and found that while tests were running and doing all their cross db stuff, we would get deadlocks.

Muz added some cool stuff to the adapter that allows you to run in different isolation levels. The method is run_with_isolation_level and it takes a block. In our application, I have a filter in our paginated results controllers, like search/aggregate controllers like this.

around_filter :with_uncommitted_isolation_level

The implementation in the private root application controller is this.

def with_uncommitted_isolation_level
  ActiveRecord::Base.connection.run_with_isolation_level('READ UNCOMMITTED') { yield }
end

This means that all of our search and aggregate view controllers are much quicker in the case where other users are writing to those tables. SQL Server waits for any object in transaction in the whole table before a SELECT happens. This totally threw me when I first learned about it. But the :lock option and Muz's addition in the adapter allows us to get around this easily and be kinder on the DB.