Lock-lesser transitions without additional queries

#350

@arthurnn opened #350 to reduce the impact of Statesman taking gap locks
when using MySQL. The crux of the issue is that MySQL's implementation
of REPEATABLE READ can take wide locks when an update touches no rows,
which happens frequently on the first transition of Statesman.

By first creating the new transition, we can avoid issuing an update
that will take the large gap lock. This order of queries meant we added
an additional query to the transition step which could impact people who
rely on low-latency Statesman transitions.

This commit is another take on the same approach that collapses two
queries into one, by taking the update of the old and new transition's
most_recent column and updating them together. It's slightly janky but
if robust, would be a good alternative to avoid additional latency.

lib/statesman/adapters/active_record.rb

@@ -65,26 +65,36 @@ def last(force_reload: false)
 
       private
 
+      # rubocop:disable Metrics/MethodLength
       def create_transition(from, to, metadata)
         transition = transitions_for_parent.build(
           default_transition_attributes(to, metadata),
         )
 
         ::ActiveRecord::Base.transaction(requires_new: true) do
           @observer.execute(:before, from, to, transition)
-          # We save the transition first, and mark it as
-          # most_recent after to avoid letting MySQL put a
-          # next-key lock which could cause deadlocks.
+
+          # We save the transition first with most_recent falsy, then mark most_recent
+          # true after to avoid letting MySQL acquire a next-key lock which can cause
+          # deadlocks.
+          #
+          # To avoid an additional query, we manually adjust the most_recent attribute on
+          # our transition assuming that update_most_recents will have set it to true.
           transition.save!
-          unset_old_most_recent
-          transition.update!(most_recent: true)
+          unless update_most_recents(transition.id) > 0
+            raise ActiveRecord::Rollback, "failed to update most_recent"
+          end
+
+          transition.assign_attributes(most_recent: true)
+
           @last_transition = transition
           @observer.execute(:after, from, to, transition)
           add_after_commit_callback(from, to, transition)
         end
 
         transition
       end
+      # rubocop:enable Metrics/MethodLength
 
       def default_transition_attributes(to, metadata)
         transition_attributes = { to_state: to,
@@ -112,21 +122,58 @@ def transitions_for_parent
         parent_model.send(@association_name)
       end
 
-      def unset_old_most_recent
-        most_recent = transitions_for_parent.where(most_recent: true)
+      # Sets the given transition most_recent = t while unsetting the most_recent of any
+      # previous transitions.
+      def update_most_recents(most_recent_id)
+        transitions = transitions_for_parent
+        last_or_current = transitions.where(id: most_recent_id).or(
+          transitions.where(most_recent: true)
+        )
 
-        # Check whether the `most_recent` column allows null values. If it
-        # doesn't, set old records to `false`, otherwise, set them to `NULL`.
-        #
-        # Some conditioning here is required to support databases that don't
-        # support partial indexes. By doing the conditioning on the column,
-        # rather than Rails' opinion of whether the database supports partial
-        # indexes, we're robust to DBs later adding support for partial indexes.
-        if transition_class.columns_hash["most_recent"].null == false
-          most_recent.update_all(with_updated_timestamp(most_recent: false))
-        else
-          most_recent.update_all(with_updated_timestamp(most_recent: nil))
+        last_or_current.update_all(
+          build_most_recents_update_all(most_recent_id),
+        )
+      end
+
+      # Generates update_all parameters that will touch the updated timestamp (if valid
+      # for this model) and ensure only the transition with the most_recent_id has
+      # most_recent set to true.
+      #
+      # This is quite nasty, but combines two updates (set all most_recent = f, set
+      # current most_recent = t) into one, which helps improve transition performance
+      # especially when database latency is significant.
+      #
+      # The SQL this can help produce looks like:
+      #
+      #   update transitions
+      #      set most_recent = (case when id = 'PA123' then TRUE else FALSE end)
+      #        , updated_at = '...'
+      #      ...
+      #
+      def build_most_recents_update_all(most_recent_id)
+        clause = "most_recent = (case when id = ? then ? else ? end)"
+        parameters = [most_recent_id, true, not_most_recent_value]
+
+        updated_column, updated_at = updated_timestamp
+        if updated_column
+          clause += ", #{updated_column} = ?"
+          parameters.push(updated_at)
         end
+
+        [clause, *parameters]
+      end
+
+      # Check whether the `most_recent` column allows null values. If it doesn't, set old
+      # records to `false`, otherwise, set them to `NULL`.
+      #
+      # Some conditioning here is required to support databases that don't support partial
+      # indexes. By doing the conditioning on the column, rather than Rails' opinion of
+      # whether the database supports partial indexes, we're robust to DBs later adding
+      # support for partial indexes.
+      def not_most_recent_value
+        return false if transition_class.columns_hash["most_recent"].null == false
+
+        nil
       end
 
       def next_sort_key
@@ -184,7 +231,8 @@ def association_join_primary_key(association)
         end
       end
 
-      def with_updated_timestamp(params)
+      # updated_timestamp should return [column_name, value]
+      def updated_timestamp
         # TODO: Once we've set expectations that transition classes should conform to
         # the interface of Adapters::ActiveRecordTransition as a breaking change in the
         # next major version, we can stop calling `#respond_to?` first and instead
@@ -198,15 +246,12 @@ def with_updated_timestamp(params)
                    ActiveRecordTransition::DEFAULT_UPDATED_TIMESTAMP_COLUMN
                  end
 
-        return params if column.nil?
-
-        timestamp = if ::ActiveRecord::Base.default_timezone == :utc
-                      Time.now.utc
-                    else
-                      Time.now
-                    end
+        # No updated timestamp column, don't return anything
+        return nil if column.nil?
 
-        params.merge(column => timestamp)
+        [
+          column, ::ActiveRecord::Base.default_timezone == :utc ? Time.now.utc : Time.now,
+        ]
       end
     end