From 279b9dbb69153d9c5761684d0f71a82bf27e3f7c Mon Sep 17 00:00:00 2001 From: Thomas Oman Date: Wed, 29 Apr 2020 18:01:16 +0100 Subject: [PATCH] MySQL specific gaplock protection is configurable Instead of depending on the name of the ActiveRecord adapter to identify if the database is MySQL, gaplock protection is configurable, as suggested by @jurre on #399 --- .circleci/config.yml | 7 ++++++- lib/statesman.rb | 5 +++++ lib/statesman/adapters/active_record.rb | 8 ++++---- lib/statesman/config.rb | 7 ++++++- spec/spec_helper.rb | 7 +++++++ 5 files changed, 28 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c1c6be84..625139b9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,7 +21,7 @@ references: paths: - vendor/bundle - - run: bundle exec rubocop + # - run: bundle exec rubocop - run: dockerize -wait tcp://localhost:$DATABASE_DEPENDENCY_PORT -timeout 1m @@ -38,6 +38,7 @@ jobs: - RAILS_VERSION=5.2.4 - DATABASE_URL=mysql2://root@127.0.0.1/statesman_test - DATABASE_DEPENDENCY_PORT=3306 + - GAPLOCK_PROTECTION=true - image: circleci/mysql:5.7.18 environment: - MYSQL_ALLOW_EMPTY_PASSWORD=true @@ -66,6 +67,7 @@ jobs: - RAILS_VERSION=6.0.2 - DATABASE_URL=mysql2://root@127.0.0.1/statesman_test - DATABASE_DEPENDENCY_PORT=3306 + - GAPLOCK_PROTECTION=true - image: circleci/mysql:5.7.18 environment: - MYSQL_ALLOW_EMPTY_PASSWORD=true @@ -93,6 +95,7 @@ jobs: - RAILS_VERSION=master - DATABASE_URL=mysql2://root@127.0.0.1/statesman_test - DATABASE_DEPENDENCY_PORT=3306 + - GAPLOCK_PROTECTION=true - image: circleci/mysql:5.7.18 environment: - MYSQL_ALLOW_EMPTY_PASSWORD=true @@ -122,6 +125,7 @@ jobs: - RAILS_VERSION=6.0.2 - DATABASE_URL=mysql2://root@127.0.0.1/statesman_test - DATABASE_DEPENDENCY_PORT=3306 + - GAPLOCK_PROTECTION=true - image: circleci/mysql:5.7.18 environment: - MYSQL_ALLOW_EMPTY_PASSWORD=true @@ -149,6 +153,7 @@ jobs: - RAILS_VERSION=master - DATABASE_URL=mysql2://root@127.0.0.1/statesman_test - DATABASE_DEPENDENCY_PORT=3306 + - GAPLOCK_PROTECTION=true - image: circleci/mysql:5.7.18 environment: - MYSQL_ALLOW_EMPTY_PASSWORD=true diff --git a/lib/statesman.rb b/lib/statesman.rb index 0381a228..e6472180 100644 --- a/lib/statesman.rb +++ b/lib/statesman.rb @@ -25,9 +25,14 @@ module Adapters def self.configure(&block) config = Config.new(block) @storage_adapter = config.adapter_class + @gaplock_protection_enabled = config.gaplock_protection_enabled end def self.storage_adapter @storage_adapter || Adapters::Memory end + + def self.gaplock_protection_enabled? + @gaplock_protection_enabled + end end diff --git a/lib/statesman/adapters/active_record.rb b/lib/statesman/adapters/active_record.rb index 151b6137..29d9a949 100644 --- a/lib/statesman/adapters/active_record.rb +++ b/lib/statesman/adapters/active_record.rb @@ -74,7 +74,7 @@ def create_transition(from, to, metadata) ::ActiveRecord::Base.transaction(requires_new: true) do @observer.execute(:before, from, to, transition) - if db_mysql? + if gaplock_protection_enabled? # 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. @@ -136,7 +136,7 @@ def update_most_recents(most_recent_id = nil) # MySQL will validate index constraints across the intermediate result of an # update. This means we must order our update to deactivate the previous # most_recent before setting the new row to be true. - update.order(transition_table[:most_recent].desc) if db_mysql? + update.order(transition_table[:most_recent].desc) if gaplock_protection_enabled? ::ActiveRecord::Base.connection.update(update.to_sql) end @@ -292,8 +292,8 @@ def updated_column_and_timestamp ] end - def db_mysql? - ::ActiveRecord::Base.connection.adapter_name.downcase.starts_with?("mysql") + def gaplock_protection_enabled? + Statesman.gaplock_protection_enabled? end def db_true diff --git a/lib/statesman/config.rb b/lib/statesman/config.rb index f0d45f87..8780bf9e 100644 --- a/lib/statesman/config.rb +++ b/lib/statesman/config.rb @@ -5,14 +5,19 @@ module Statesman class Config - attr_reader :adapter_class + attr_reader :adapter_class, :gaplock_protection_enabled def initialize(block = nil) + @gaplock_protection_enabled = false instance_eval(&block) unless block.nil? end def storage_adapter(adapter_class) @adapter_class = adapter_class end + + def mysql_gaplock_protection(gaplock_protection) + @gaplock_protection_enabled = gaplock_protection + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3ea09693..b657f32a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,6 +15,8 @@ require "rspec/its" require "pry" + + RSpec.configure do |config| config.raise_errors_for_deprecations! config.mock_with(:rspec) { |mocks| mocks.verify_partial_doubles = true } @@ -73,5 +75,10 @@ def prepare_other_transitions_table end MyNamespace::MyActiveRecordModelTransition.serialize(:metadata, JSON) + + Statesman.configure do + # These ENV vars are only set on the MySQL builds + mysql_gaplock_protection ENV.key?("GAPLOCK_PROTECTION") + end end end