brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

acts_as_list_class.maximum(position_column) is causing the entire table to lock #264

Closed yatish27 closed 7 years ago

yatish27 commented 7 years ago
def insert_at_position(position)
  return set_list_position(position) if new_record?
  with_lock do
    if in_list?
      old_position = send(position_column).to_i
      return if position == old_position
      # temporary move after bottom with gap, avoiding duplicate values
      # gap is required to leave room for position increments
      # positive number will be valid with unique not null check (>= 0) db constraint
      temporary_position = acts_as_list_class.maximum(position_column).to_i + 2
      set_list_position(temporary_position)
      shuffle_positions_on_intermediate_items(old_position, position, id)
    else
      increment_positions_on_lower_items(position)
    end
    set_list_position(position)
  end
end

This line is causing to load/locking the entire table. It causes the following query:

SELECT MAX(`items`.`position`) FROM `items`

This is a huge performance issue, if the table is large. We should either remove the temporary position setting or use scope which is attached to acts_as_list for that class.

Maybe we can also add acts_as_list_class.where(scope_condition) before calling maximum.

swanandp commented 7 years ago

Updated original comment with slightly better formatting

swanandp commented 7 years ago

@yatish27 Either @brendon or I will look into this shortly.

yatish27 commented 7 years ago

@swanandp @brendon Any updates on this one ?

brendon commented 7 years ago

Hi @yatish27, this original change came in through another contributor's PR.

Thanks for your patience on this. Let me know if the new PR works for you.

brendon commented 7 years ago

@yatish27, this should solve your problem. Let me know if not.