Do not lock row to read unread_count

Fixes CNVS-25632

Test Plan:
Regression test discussions, especially unread
counts. We expect no behavior change.

Change-Id: I282a92491d3f15ecaacd3375e00e23acfda5eb7b
Reviewed-on: https://gerrit.instructure.com/69076
Tested-by: Jenkins
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Adrian Russell <arussell@instructure.com>
Product-Review: Matthew Wheeler <mwheeler@instructure.com>

app/models/discussion_topic.rb

@@ -388,11 +388,16 @@ class DiscussionTopic < ActiveRecord::Base
     self.discussion_entries.active.count
   end
 
-  def unread_count(current_user = nil)
+  # Do not use the lock options unless you truly need
+  # the lock, for instance to update the count.
+  # Careless use has caused database transaction deadlocks
+  def unread_count(current_user = nil, lock: false)
     current_user ||= self.current_user
     return 0 unless current_user # default for logged out users
-    Shackles.activate(:master) do
-      topic_participant = discussion_topic_participants.lock.where(user_id: current_user).select(:unread_entry_count).first
+
+    environment = lock ? :master : :slave
+    Shackles.activate(environment) do
+      topic_participant = discussion_topic_participants.where(user_id: current_user).select(:unread_entry_count).lock(lock).first
       topic_participant.try(:unread_entry_count) || self.default_unread_count
     end
   end
@@ -479,7 +484,7 @@ class DiscussionTopic < ActiveRecord::Base
         DiscussionTopic.unique_constraint_retry do
           topic_participant = self.discussion_topic_participants.where(:user_id => current_user).lock.first
           topic_participant ||= self.discussion_topic_participants.build(:user => current_user,
-                                                                         :unread_entry_count => self.unread_count(current_user),
+                                                                         :unread_entry_count => self.unread_count(current_user, lock: true),
                                                                          :workflow_state => "unread",
                                                                          :subscribed => current_user == user && !subscription_hold(current_user, nil, nil))
           topic_participant.workflow_state = opts[:new_state] if opts[:new_state]

spec/models/discussion_topic_spec.rb

@@ -1244,6 +1244,23 @@ describe DiscussionTopic do
     end
   end
 
+  describe "#unread_count" do
+    let(:topic) do
+      @course.discussion_topics.create!(:title => "title", :message => "message")
+    end
+
+    it "returns 0 for a nil user" do
+      topic.discussion_entries.create!
+      expect(topic.unread_count(nil)).to eq 0
+    end
+
+    it "returns the default_unread_count if the user has no discussion_topic_participant" do
+      topic.discussion_entries.create!
+      student_in_course
+      expect(topic.unread_count(@student)).to eq 1
+    end
+  end
+
   context "read/unread state" do
     before(:once) do
       @topic = @course.discussion_topics.create!(:title => "title", :message => "message", :user => @teacher)