diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 468a965e..2e328cfb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -119,6 +119,7 @@ jobs: - redis_4 - redis_5 - redis_client + - activerecord_trilogy_adapter include: - gemfile: grpc adapter: grpc @@ -134,6 +135,8 @@ jobs: adapter: redis - gemfile: redis_client adapter: redis_client + - gemfile: activerecord_trilogy_adapter + adapter: activerecord_trilogy_adapter services: mysql: image: mysql:5.7 diff --git a/.rubocop.yml b/.rubocop.yml index 30704460..56156eb3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -28,6 +28,9 @@ Lint/SuppressedException: Minitest/AssertPredicate: Enabled: false +Minitest/MultipleAssertions: + Enabled: false + Minitest/RefuteFalse: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b9a651e..48ff5e94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +* Support Active Record Trilogy adapter. (#468) + # v0.17.0 * Avoid prepending the same prefix twice to errors messages. (#423) diff --git a/Gemfile b/Gemfile index af5fd6ef..fefb14ed 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,9 @@ group :test do # The last stable version for MacOS ARM darwin gem "grpc", "1.47.0" gem "mysql2", "~> 0.5" - gem "activerecord", ">= 7.0.3" + gem "trilogy", github: "github/trilogy", branch: "main", glob: "contrib/ruby/*.gemspec" + gem "activerecord", github: "rails/rails", branch: "main" + gem "activerecord-trilogy-adapter", github: "github/activerecord-trilogy-adapter", branch: "main" gem "hiredis", "~> 0.6" # NOTE: v0.12.0 required for ruby 3.2.0. https://github.com/redis-rb/redis-client/issues/58 gem "hiredis-client", ">= 0.12.0" diff --git a/Gemfile.lock b/Gemfile.lock index b5bac57c..93b13e10 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,37 @@ +GIT + remote: https://github.com/github/activerecord-trilogy-adapter.git + revision: 516c234352388c8b423f9bb50ca4d54bef8f04fa + branch: main + specs: + activerecord-trilogy-adapter (2.1.0) + activerecord (~> 7.1.a) + trilogy (>= 2.1.1) + +GIT + remote: https://github.com/github/trilogy.git + revision: 07f83dbe36c7ea31a348847eabcb8f3860a07ab0 + branch: main + glob: contrib/ruby/*.gemspec + specs: + trilogy (2.2.0) + +GIT + remote: https://github.com/rails/rails.git + revision: 03702da57aa4e607e0115ea063908f9d4df7c3a0 + branch: main + specs: + activemodel (7.1.0.alpha) + activesupport (= 7.1.0.alpha) + activerecord (7.1.0.alpha) + activemodel (= 7.1.0.alpha) + activesupport (= 7.1.0.alpha) + activesupport (7.1.0.alpha) + concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + PATH remote: . specs: @@ -6,29 +40,27 @@ PATH GEM remote: https://rubygems.org/ specs: - activemodel (7.0.4.1) - activesupport (= 7.0.4.1) - activerecord (7.0.4.1) - activemodel (= 7.0.4.1) - activesupport (= 7.0.4.1) - activesupport (7.0.4.1) - concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (>= 1.6, < 2) - minitest (>= 5.1) - tzinfo (~> 2.0) ast (2.4.2) benchmark-memory (0.2.0) memory_profiler (~> 1) byebug (11.1.3) coderay (1.1.3) - concurrent-ruby (1.1.10) + concurrent-ruby (1.2.0) connection_pool (2.3.0) google-protobuf (3.21.12) + google-protobuf (3.21.12-x86_64-darwin) + google-protobuf (3.21.12-x86_64-linux) googleapis-common-protos-types (1.5.0) google-protobuf (~> 3.14) grpc (1.47.0) google-protobuf (~> 3.19) googleapis-common-protos-types (~> 1.0) + grpc (1.47.0-x86_64-darwin) + google-protobuf (~> 3.19) + googleapis-common-protos-types (~> 1.0) + grpc (1.47.0-x86_64-linux) + google-protobuf (~> 3.19) + googleapis-common-protos-types (~> 1.0) hiredis (0.6.3) hiredis-client (0.12.1) redis-client (= 0.12.1) @@ -81,7 +113,7 @@ GEM ruby-progressbar (1.11.0) ruby2_keywords (0.0.5) toxiproxy (2.0.2) - tzinfo (2.0.5) + tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (2.4.2) webrick (1.7.0) @@ -94,7 +126,8 @@ PLATFORMS x86_64-linux DEPENDENCIES - activerecord (>= 7.0.3) + activerecord! + activerecord-trilogy-adapter! benchmark-memory grpc (= 1.47.0) hiredis (~> 0.6) @@ -113,6 +146,7 @@ DEPENDENCIES rubocop-shopify semian! toxiproxy + trilogy! webrick BUNDLED WITH diff --git a/README.md b/README.md index b702fa0a..adf368cb 100644 --- a/README.md +++ b/README.md @@ -859,6 +859,7 @@ $ bundle install [mysql-semian-adapter]: lib/semian/mysql2.rb [postgres-semian-adapter]: https://github.com/mschoenlaub/semian-postgres [redis-semian-adapter]: lib/semian/redis.rb +[activerecord-trilogy-semian-adapter]: lib/semian/activerecord_trilogy_adapter.rb [semian-adapter]: lib/semian/adapter.rb [nethttp-semian-adapter]: lib/semian/net_http.rb [nethttp-default-errors]: lib/semian/net_http.rb#L35-L45 diff --git a/gemfiles/activerecord_trilogy_adapter.gemfile b/gemfiles/activerecord_trilogy_adapter.gemfile new file mode 100644 index 00000000..7e2a5ccf --- /dev/null +++ b/gemfiles/activerecord_trilogy_adapter.gemfile @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +gem "rake" +gem "rake-compiler" +gem "minitest" +gem "mocha" +gem "toxiproxy" +gem "webrick" + +gem "trilogy", github: "github/trilogy", branch: "main", glob: "contrib/ruby/*.gemspec" +gem "activerecord-trilogy-adapter", github: "github/activerecord-trilogy-adapter", branch: "main" +gem "activerecord", github: "rails/rails" + +gemspec path: "../" diff --git a/gemfiles/activerecord_trilogy_adapter.gemfile.lock b/gemfiles/activerecord_trilogy_adapter.gemfile.lock new file mode 100644 index 00000000..362e2545 --- /dev/null +++ b/gemfiles/activerecord_trilogy_adapter.gemfile.lock @@ -0,0 +1,74 @@ +GIT + remote: https://github.com/github/activerecord-trilogy-adapter.git + revision: 516c234352388c8b423f9bb50ca4d54bef8f04fa + branch: main + specs: + activerecord-trilogy-adapter (2.1.0) + activerecord (~> 7.1.a) + trilogy (>= 2.1.1) + +GIT + remote: https://github.com/github/trilogy.git + revision: 2dd91e5d1974b29473ad646e5dba0a50562fe37a + branch: main + glob: contrib/ruby/*.gemspec + specs: + trilogy (2.2.0) + +GIT + remote: https://github.com/rails/rails.git + revision: 6017df6a8e1468eef6432065e7a679f7f203827f + specs: + activemodel (7.1.0.alpha) + activesupport (= 7.1.0.alpha) + activerecord (7.1.0.alpha) + activemodel (= 7.1.0.alpha) + activesupport (= 7.1.0.alpha) + activesupport (7.1.0.alpha) + concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + +PATH + remote: .. + specs: + semian (0.16.0) + +GEM + remote: https://rubygems.org/ + specs: + concurrent-ruby (1.2.0) + connection_pool (2.3.0) + i18n (1.12.0) + concurrent-ruby (~> 1.0) + minitest (5.17.0) + mocha (2.0.2) + ruby2_keywords (>= 0.0.5) + rake (13.0.6) + rake-compiler (1.2.1) + rake + ruby2_keywords (0.0.5) + toxiproxy (2.0.2) + tzinfo (2.0.6) + concurrent-ruby (~> 1.0) + webrick (1.8.1) + +PLATFORMS + arm64-darwin-21 + +DEPENDENCIES + activerecord! + activerecord-trilogy-adapter! + minitest + mocha + rake + rake-compiler + semian! + toxiproxy + trilogy! + webrick + +BUNDLED WITH + 2.4.3 diff --git a/lib/semian/activerecord_trilogy_adapter.rb b/lib/semian/activerecord_trilogy_adapter.rb new file mode 100644 index 00000000..5c42fd6c --- /dev/null +++ b/lib/semian/activerecord_trilogy_adapter.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require "semian/adapter" +require "activerecord-trilogy-adapter" +require "active_record/connection_adapters/trilogy_adapter" + +module ActiveRecord + module ConnectionAdapters + class TrilogyAdapter + ActiveRecord::ActiveRecordError.include(::Semian::AdapterError) + + class SemianError < StatementInvalid + def initialize(semian_identifier, *args) + super(*args) + @semian_identifier = semian_identifier + end + end + + ResourceBusyError = Class.new(SemianError) + CircuitOpenError = Class.new(SemianError) + end + end +end + +module Semian + module ActiveRecordTrilogyAdapter + include Semian::Adapter + + ResourceBusyError = ::ActiveRecord::ConnectionAdapters::TrilogyAdapter::ResourceBusyError + CircuitOpenError = ::ActiveRecord::ConnectionAdapters::TrilogyAdapter::CircuitOpenError + + attr_reader :raw_semian_options, :semian_identifier + + def initialize(*options) + *, config = options + @raw_semian_options = config.delete(:semian) + @semian_identifier = begin + name = semian_options && semian_options[:name] + unless name + host = config[:host] || "localhost" + port = config[:port] || 3306 + name = "#{host}:#{port}" + end + :"mysql_#{name}" + end + super + end + + def execute(sql, name = nil, async: false, allow_retry: false) + if query_allowlisted?(sql) + super(sql, name, async: async, allow_retry: allow_retry) + else + acquire_semian_resource(adapter: :trilogy_adapter, scope: :execute) do + super(sql, name, async: async, allow_retry: allow_retry) + end + end + end + + def active? + acquire_semian_resource(adapter: :trilogy_adapter, scope: :ping) do + super + end + rescue ResourceBusyError, CircuitOpenError + false + end + + def with_resource_timeout(temp_timeout) + if connection.nil? + prev_read_timeout = @config[:read_timeout] || 0 + @config.merge!(read_timeout: temp_timeout) # Create new client with temp_timeout for read timeout + else + prev_read_timeout = connection.read_timeout + connection.read_timeout = temp_timeout + end + yield + ensure + @config.merge!(read_timeout: prev_read_timeout) + connection&.read_timeout = prev_read_timeout + end + + private + + def acquire_semian_resource(**) + super + rescue ActiveRecord::StatementInvalid => error + if error.cause.is_a?(Trilogy::TimeoutError) + semian_resource.mark_failed(error) + error.semian_identifier = semian_identifier + end + raise + end + + def resource_exceptions + [ActiveRecord::ConnectionNotEstablished] + end + + # TODO: share this with Mysql2 + QUERY_ALLOWLIST = Regexp.union( + %r{\A(?:/\*.*?\*/)?\s*ROLLBACK}i, + %r{\A(?:/\*.*?\*/)?\s*COMMIT}i, + %r{\A(?:/\*.*?\*/)?\s*RELEASE\s+SAVEPOINT}i, + ) + + def query_allowlisted?(sql, *) + QUERY_ALLOWLIST.match?(sql) + rescue ArgumentError + return false unless sql.valid_encoding? + + raise + end + + def connect(*args) + acquire_semian_resource(adapter: :trilogy_adapter, scope: :connection) do + super + end + end + end +end + +ActiveRecord::ConnectionAdapters::TrilogyAdapter.prepend(Semian::ActiveRecordTrilogyAdapter) diff --git a/test/adapters/activerecord_trilogy_adapter_test.rb b/test/adapters/activerecord_trilogy_adapter_test.rb new file mode 100644 index 00000000..b1d1f38e --- /dev/null +++ b/test/adapters/activerecord_trilogy_adapter_test.rb @@ -0,0 +1,406 @@ +# frozen_string_literal: true + +require "test_helper" +require "semian/activerecord_trilogy_adapter" + +module ActiveRecord + module ConnectionAdapters + class ActiveRecordTrilogyAdapterTest < Minitest::Test + ERROR_TIMEOUT = 5 + ERROR_THRESHOLD = 1 + SEMIAN_OPTIONS = { + name: "testing", + tickets: 1, + timeout: 0, + error_threshold: ERROR_THRESHOLD, + success_threshold: 2, + error_timeout: ERROR_TIMEOUT, + } + + def setup + @proxy = Toxiproxy[:semian_test_mysql] + Semian.destroy(:mysql_testing) + + @configuration = { + adapter: "trilogy", + username: "root", + host: SemianConfig["toxiproxy_upstream_host"], + port: SemianConfig["mysql_toxiproxy_port"], + read_timeout: 2, + write_timeout: 2, + semian: SEMIAN_OPTIONS, + } + @adapter = trilogy_adapter + end + + def teardown + @adapter.disconnect! + end + + def test_semian_identifier + assert_equal(:mysql_testing, @adapter.semian_identifier) + + adapter = trilogy_adapter(host: "127.0.0.1", semian: { name: nil }) + + assert_equal(:"mysql_127.0.0.1:13306", adapter.semian_identifier) + + adapter = trilogy_adapter(host: "example.com", port: 42, semian: { name: nil }) + + assert_equal(:"mysql_example.com:42", adapter.semian_identifier) + end + + def test_semian_can_be_disabled + resource = trilogy_adapter( + host: SemianConfig["toxiproxy_upstream_host"], + port: SemianConfig["mysql_toxiproxy_port"], + semian: false, + ).semian_resource + + assert_instance_of(Semian::UnprotectedResource, resource) + end + + def test_unconfigured + adapter = trilogy_adapter( + host: SemianConfig["toxiproxy_upstream_host"], + port: SemianConfig["mysql_toxiproxy_port"], + ) + + assert_equal(2, adapter.execute("SELECT 1 + 1;").to_a.flatten.first) + end + + def test_connection_errors_open_the_circuit + @proxy.downstream(:latency, latency: 2200).apply do + ERROR_THRESHOLD.times do + assert_raises(ActiveRecord::ConnectionNotEstablished) do + @adapter.execute("SELECT 1;") + end + end + + assert_raises(TrilogyAdapter::CircuitOpenError) do + @adapter.execute("SELECT 1;") + end + end + end + + def test_query_errors_do_not_open_the_circuit + ERROR_THRESHOLD.times do + assert_raises(ActiveRecord::StatementInvalid) do + @adapter.execute("ERROR!") + end + end + err = assert_raises(ActiveRecord::StatementInvalid) do + @adapter.execute("ERROR!") + end + + refute_kind_of(TrilogyAdapter::CircuitOpenError, err) + end + + def test_read_timeout_error_opens_the_circuit + ERROR_THRESHOLD.times do + assert_raises(ActiveRecord::StatementInvalid) do + @adapter.execute("SELECT sleep(5)") + end + end + + assert_raises(TrilogyAdapter::CircuitOpenError) do + @adapter.execute("SELECT sleep(5)") + end + + # After TrilogyAdapter::CircuitOpenError check regular queries are working fine. + result = time_travel(ERROR_TIMEOUT + 1) do + @adapter.execute("SELECT 1 + 1;") + end + + assert_equal(2, result.first[0]) + end + + def test_connect_instrumentation + notified = false + subscriber = Semian.subscribe do |event, resource, scope, adapter| + next unless event == :success + + notified = true + + assert_equal(Semian[:mysql_testing], resource) + assert_equal(:connection, scope) + assert_equal(:trilogy_adapter, adapter) + end + + # We can't use the public #connect! API here because we'll call + # active?, which will scope the event to :ping. + @adapter.send(:connect) + + assert(notified, "No notifications have been emitted") + ensure + Semian.unsubscribe(subscriber) + end + + def test_query_instrumentation + @adapter.connect! + + notified = false + subscriber = Semian.subscribe do |event, resource, scope, adapter| + notified = true + + assert_equal(:success, event) + assert_equal(Semian[:mysql_testing], resource) + assert_equal(:execute, scope) + assert_equal(:trilogy_adapter, adapter) + end + + @adapter.execute("SELECT 1;") + + assert(notified, "No notifications has been emitted") + ensure + Semian.unsubscribe(subscriber) + end + + def test_active_instrumentation + @adapter.connect! + + notified = false + subscriber = Semian.subscribe do |event, resource, scope, adapter| + notified = true + + assert_equal(:success, event) + assert_equal(Semian[:mysql_testing], resource) + assert_equal(:ping, scope) + assert_equal(:trilogy_adapter, adapter) + end + + @adapter.active? + + assert(notified, "No notifications has been emitted") + ensure + Semian.unsubscribe(subscriber) + end + + def test_network_errors_are_tagged_with_the_resource_identifier + @proxy.down do + error = assert_raises(ActiveRecord::ConnectionNotEstablished) do + @adapter.execute("SELECT 1 + 1;") + end + + assert_equal(@adapter.semian_identifier, error.semian_identifier) + end + end + + def test_other_mysql_errors_are_not_tagged_with_the_resource_identifier + error = assert_raises(ActiveRecord::StatementInvalid) do + @adapter.execute("SYNTAX ERROR!") + end + + assert_nil(error.semian_identifier) + end + + def test_resource_acquisition_for_connect + @adapter.connect! + + Semian[:mysql_testing].acquire do + error = assert_raises(TrilogyAdapter::ResourceBusyError) do + trilogy_adapter.send(:connect) # Avoid going through connect!, which will call #active? + end + + assert_equal(:mysql_testing, error.semian_identifier) + end + end + + def test_resource_acquisition_for_query + @adapter.connect! + + Semian[:mysql_testing].acquire do + assert_raises(TrilogyAdapter::ResourceBusyError) do + @adapter.execute("SELECT 1;") + end + end + end + + def test_resource_timeout_on_connect + @proxy.downstream(:latency, latency: 500).apply do + background { @adapter.connect! } + + assert_raises(TrilogyAdapter::ResourceBusyError) do + trilogy_adapter.send(:connect) # Avoid going through connect!, which will call #active? + end + end + end + + def test_circuit_breaker_on_connect + @proxy.downstream(:latency, latency: 500).apply do + background { @adapter.connect! } + + ERROR_THRESHOLD.times do + assert_raises(TrilogyAdapter::ResourceBusyError) do + trilogy_adapter.send(:connect) # Avoid going through connect!, which will call #active? + end + end + end + + yield_to_background + + assert_raises(TrilogyAdapter::CircuitOpenError) do + trilogy_adapter.connect! + end + + time_travel(ERROR_TIMEOUT + 1) do + trilogy_adapter.connect! + end + end + + def test_resource_timeout_on_query + adapter2 = trilogy_adapter + + @proxy.downstream(:latency, latency: 500).apply do + background { adapter2.execute("SELECT 1 + 1;") } + + assert_raises(TrilogyAdapter::ResourceBusyError) do + @adapter.query("SELECT 1 + 1;") + end + end + end + + def test_circuit_breaker_on_query + @proxy.downstream(:latency, latency: 2200).apply do + background { trilogy_adapter.execute("SELECT 1 + 1;") } + + ERROR_THRESHOLD.times do + assert_raises(TrilogyAdapter::ResourceBusyError) do + @adapter.query("SELECT 1 + 1;") + end + end + end + + yield_to_background + + assert_raises(TrilogyAdapter::CircuitOpenError) do + @adapter.execute("SELECT 1 + 1;") + end + + time_travel(ERROR_TIMEOUT + 1) do + assert_equal(2, @adapter.execute("SELECT 1 + 1;").to_a.flatten.first) + end + end + + def test_semian_allows_rollback + @adapter.execute("START TRANSACTION;") + + Semian[:mysql_testing].acquire do + @adapter.execute("ROLLBACK;") + end + end + + def test_semian_allows_rollback_with_marginalia + @adapter.execute("START TRANSACTION;") + + Semian[:mysql_testing].acquire do + @adapter.execute("/*foo:bar*/ ROLLBACK;") + end + end + + def test_semian_allows_commit + @adapter.execute("START TRANSACTION;") + + Semian[:mysql_testing].acquire do + @adapter.execute("COMMIT;") + end + end + + def test_query_allowlisted_returns_false_for_binary_sql + binary_query = File.read(File.expand_path("../../fixtures/binary.sql", __FILE__)) + + refute(@adapter.send(:query_allowlisted?, binary_query)) + end + + def test_semian_allows_rollback_to_safepoint + @adapter.execute("START TRANSACTION;") + @adapter.execute("SAVEPOINT foobar;") + + Semian[:mysql_testing].acquire do + @adapter.execute("ROLLBACK TO foobar;") + end + + @adapter.execute("ROLLBACK;") + end + + def test_semian_allows_release_savepoint + @adapter.execute("START TRANSACTION;") + @adapter.execute("SAVEPOINT foobar;") + + Semian[:mysql_testing].acquire do + @adapter.execute("RELEASE SAVEPOINT foobar;") + end + + @adapter.execute("ROLLBACK;") + end + + def test_changes_timeout_when_half_open_and_configured + adapter = trilogy_adapter(semian: SEMIAN_OPTIONS.merge(half_open_resource_timeout: 1)) + + @proxy.downstream(:latency, latency: 3000).apply do + ERROR_THRESHOLD.times do + assert_raises(ActiveRecord::ConnectionNotEstablished) do + adapter.execute("SELECT 1 + 1;") + end + end + end + + assert_raises(TrilogyAdapter::CircuitOpenError) do + adapter.execute("SELECT 1 + 1;") + end + + # Circuit moves to half-open state, so 1500 of latency should result in error + time_travel(ERROR_TIMEOUT + 1) do + @proxy.downstream(:latency, latency: 1500).apply do + assert_raises(ActiveRecord::ConnectionNotEstablished) do + adapter.execute("SELECT 1 + 1;") + end + end + end + + time_travel(ERROR_TIMEOUT * 2 + 1) do + adapter.execute("SELECT 1 + 1;") + adapter.execute("SELECT 1 + 1;") + + # Timeout has reset to the normal 2 seconds now that circuit is closed + @proxy.downstream(:latency, latency: 1500).apply do + adapter.execute("SELECT 1 + 1;") + end + end + + raw_connection = adapter.send(:connection) + + assert_equal(2, raw_connection.read_timeout) + assert_equal(2, raw_connection.write_timeout) + end + + def test_trilogy_default_read_timeout + client = Trilogy.new(@configuration.slice(:username, :host, :port)) + + assert_equal(0, client.read_timeout) + end + + def test_circuit_open_errors_do_not_trigger_the_circuit_breaker + @proxy.down do + ERROR_THRESHOLD.times do + assert_raises(ActiveRecord::ConnectionNotEstablished) do + @adapter.execute("SELECT 1;") + end + end + + assert_raises(TrilogyAdapter::CircuitOpenError) do + @adapter.execute("SELECT 1;") + end + error = Semian[:mysql_testing].circuit_breaker.last_error + + assert_equal(ActiveRecord::ConnectionNotEstablished, error.class) + end + end + + private + + def trilogy_adapter(**config_overrides) + TrilogyAdapter.new(@configuration.merge(config_overrides)) + end + end + end +end