Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
yatish27 opened this issue Mar 29, 2017 · 5 comments

Comments

@yatish27
Copy link

yatish27 commented Mar 29, 2017

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
Copy link
Contributor

Updated original comment with slightly better formatting

@swanandp
Copy link
Contributor

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

@yatish27
Copy link
Author

@swanandp @brendon Any updates on this one ?

@brendon
Copy link
Owner

brendon commented Apr 1, 2017

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
Copy link
Owner

brendon commented Apr 2, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants