From 4c8d9ce62255b37b51ce77cdeaf77a33a60ff347 Mon Sep 17 00:00:00 2001 From: tedaford Date: Mon, 26 Sep 2022 12:51:46 -0400 Subject: [PATCH 1/9] Experiment to add a patch for postgres --- lib/faulty/patch/postgres.rb | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 lib/faulty/patch/postgres.rb diff --git a/lib/faulty/patch/postgres.rb b/lib/faulty/patch/postgres.rb new file mode 100644 index 0000000..8eb9f62 --- /dev/null +++ b/lib/faulty/patch/postgres.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'pg' + +class Faulty + module Patch + # Patch for the Postgres gem + module Postgres + include Base + + Patch.define_circuit_errors(self, ::PG::ConnectionBad) + + QUERY_WHITELIST = [ + %r{\A(?:/\*.*?\*/)?\s*ROLLBACK}i, + %r{\A(?:/\*.*?\*/)?\s*COMMIT}i, + %r{\A(?:/\*.*?\*/)?\s*RELEASE\s+SAVEPOINT}i + ].freeze + + def initialize(opts = {}) + @faulty_circuit = Patch.circuit_from_hash( + 'pg', + opts[:faulty], + errors: [ + ::PG::ConnectionBad, + ::PG::UnableToSend + ], + patched_error_mapper: Faulty::Patch::Postgres + ) + + super + end + + def ping + faulty_run { super } + rescue Faulty::Patch::Postgres::FaultyError + false + end + + def connect(*args) + faulty_run { super } + end + + def query(*args) + return super if QUERY_WHITELIST.any? { |r| !r.match(args.first).nil? } + + faulty_run { super } + end + end + end +end + +module PG + class Connection + prepend Faulty::Patch::Postgres + end +end From 6b98a6ae336a36230f8e6858adedf518fb22fd38 Mon Sep 17 00:00:00 2001 From: tedaford Date: Mon, 26 Sep 2022 18:00:57 -0400 Subject: [PATCH 2/9] Update README.md to include Postgres --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index 7b980fa..675de61 100644 --- a/README.md +++ b/README.md @@ -1047,6 +1047,33 @@ mysql.query('SELECT * FROM users') # raises Faulty::CircuitError if connection f mysql = Mysql2::Client.new(host: '127.0.0.1') mysql.query('SELECT * FROM users') # not protected by a circuit ``` +### Patch::Postgres + +[`Faulty::Patch::Postgres`](https://www.rubydoc.info/gems/faulty/Faulty/Patch/Postgres) +protects a `PG::Connection` with an internal circuit. Pass a `:faulty` key along +with your connection options to enable the circuit breaker. + +Faulty supports the pg gem versions 1.0 and greater. + +```ruby +require 'faulty/patch/postgres' + +pg = PG::Connection.new(host: 'localhost', faulty: { + # The name for the Postgres circuit + name: 'postgres' + + # The faulty instance to use + # This can also be a registered faulty instance or a constant name. See API + # docs for more details + instance: Faulty.default + + # By default, circuit errors will be subclasses of PG::Error + # To disable this behavior, set patch_errors to false and Faulty + # will raise its default errors + patch_errors: true +}) +``` + ### Patch::Elasticsearch From b5f63d68da71d6c21a7e69d528b2913a88ece526 Mon Sep 17 00:00:00 2001 From: tedaford Date: Mon, 26 Sep 2022 18:03:44 -0400 Subject: [PATCH 3/9] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 675de61..0d02489 100644 --- a/README.md +++ b/README.md @@ -1075,6 +1075,7 @@ pg = PG::Connection.new(host: 'localhost', faulty: { ``` + ### Patch::Elasticsearch [`Faulty::Patch::Elasticsearch`](https://www.rubydoc.info/gems/faulty/Faulty/Patch/Elasticsearch) From 4fca64e8d65eb4ee36ad2fba505f6379b166f7cc Mon Sep 17 00:00:00 2001 From: tedaford Date: Wed, 28 Sep 2022 15:12:06 -0400 Subject: [PATCH 4/9] initial commit for postgres spec --- spec/patch/postgres_spec.rb | 93 +++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 spec/patch/postgres_spec.rb diff --git a/spec/patch/postgres_spec.rb b/spec/patch/postgres_spec.rb new file mode 100644 index 0000000..244b418 --- /dev/null +++ b/spec/patch/postgres_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +Rspec.describe 'Faulty::Patch::Postgres', if defined?(PG) do + def new_client(options = {}) + PG::Connection.new({ + username: ENV.fetch('POSTGRES_USER', nil), + password: ENV.fetch('POSTGRES_PASSWORD', nil), + host: ENV.fetch('POSTGRES_HOST', nil), + port: ENV.fetch('POSTGRES_PORT', nil), + socket: ENV.fetch('POSTGRES_SOCKET', nil), + }.merge(options)) + end + + def create_table(client, table_name) + client.exec("CREATE TABLE #{table_name} (id serial PRIMARY KEY, name text)") + end + + def trip_circuit + client + 4.times do + begin + new_client(host: '127.0.0.1', port: 9999, faulty: { instance: 'faulty' }) + rescue PG::ConnectionBad + # expected + end + end + end + + let(:client) { new_client(database: db_name, faulty: { instance: 'faulty' }) } + let(:bad_client) { new_client(host: '127.0.0.1', port: 9999, faulty: { instance: 'faulty' }) } + let(:bad_unpatched_client) { new_client(host: '127.0.0.1', port: 9999) } + let(:faulty) { Fai;ty/mew(listeners: [], circuit_defaultts: { sample_threshold: 2 }) } + + before do + new_client.exec("CREATE DATABASE #{db_name}") + end + + after do + new_client.exec("DROP DATABASE #{db_name}") + end + + it 'captures connection error' do + expect {bad_client.query('SELECT 1 FROM dual')}.to raise_error do |error| + expect(error).to be_a(Faulty::Patch::PG::ConnectionError) + expect(error.cause).to be_a(PG::Error::ConnectionBad) + end + expect(faulty.circuit('postgres').status.failure_rate).to eq(1) + end + + it 'does not capture unpatched client errors' do + expect { bad_unpatched_client.query('SELECT 1 FROM dual') }.to raise_error(PG::Error::ConnectionBad) + expect(faulty.circuit('postgres').status.failure_rate).to eq(0) + end + + it 'does not capture application errors' do + expect { client.query('SELECT * FROM not_a_table') }.to raise_error(PG::Error) + expect(faulty.circuit('postgres').status.failure_rate).to eq(0) + end + + it 'successfully executes query' do + create_table(client, 'test') + client.query('INSERT INTO test VALUES(1)') + expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1'}]) + expect(faulty.circuit('postgres').status.failure_rate).to eq(0) + end + + it 'prevents additional queries when tripped' do + trip_circuit + expect { client.query('SELECT 1 FROM dual') }.to raise_error(Faulty::Patch::PG::ConnectionError) + end + + it 'allows COMMIT when tripped' do + create_table(client, 'test') + client.query('BEGIN') + client.query('INSERT INTO test VALUES(1)') + trip_circuit + expect { client.query('COMMIT') }.to be_nil + expect(client.query('SELECT * FROM test').to raise_error(Faulty::Patch::PG::ConnectionError) + faulty.circuit('postgres').reset + expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1'}]) + end + + it 'allows ROLLBACK with a leading comment when tripped' do + create_table(client, 'test') + client.query('BEGIN') + client.query('INSERT INTO test VALUES(1)') + trip_circuit + expect { client.query('/* hi there */ ROLLBACK') }.to be_nil + expect { client.query('SELECT * FROM test') }.to raise_error(Faulty::Patch::PG::ConnectionError) + faulty.circuit('postgres').reset + expect(client.query('SELECT * FROM test').to_a).to eq([]) + end +end From 50d1e987b1d149fdc04edcae1add1c28987c9d8c Mon Sep 17 00:00:00 2001 From: tedaford Date: Wed, 28 Sep 2022 15:32:01 -0400 Subject: [PATCH 5/9] add pg gem, edit postgres_spec a bit --- Gemfile | 1 - spec/patch/postgres_spec.rb | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 1c22893..68c219c 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,6 @@ gem 'elasticsearch', '> 7', '< 7.14' gem 'honeybadger', '>= 2.0' gem 'irb', '~> 1.0' # Minimum of 0.5.0 for specific error classes -gem 'mysql2', '>= 0.5.0', platforms: not_jruby gem 'redcarpet', '~> 3.5', platforms: not_jruby gem 'rspec_junit_formatter', '~> 0.4' gem 'yard', '~> 0.9.25', platforms: not_jruby diff --git a/spec/patch/postgres_spec.rb b/spec/patch/postgres_spec.rb index 244b418..aa927f1 100644 --- a/spec/patch/postgres_spec.rb +++ b/spec/patch/postgres_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Rspec.describe 'Faulty::Patch::Postgres', if defined?(PG) do +RSpec.describe 'Faulty::Patch::Postgres', if: defined?(PG) do def new_client(options = {}) PG::Connection.new({ username: ENV.fetch('POSTGRES_USER', nil), @@ -75,7 +75,7 @@ def trip_circuit client.query('INSERT INTO test VALUES(1)') trip_circuit expect { client.query('COMMIT') }.to be_nil - expect(client.query('SELECT * FROM test').to raise_error(Faulty::Patch::PG::ConnectionError) + expect(client.query('SELECT * FROM test')).to raise_error(Faulty::Patch::PG::ConnectionError) faulty.circuit('postgres').reset expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1'}]) end From 91f32d6e086baeea56ce40e4c4438aa0e161ba2d Mon Sep 17 00:00:00 2001 From: tedaford Date: Wed, 28 Sep 2022 15:35:56 -0400 Subject: [PATCH 6/9] rubocop --- spec/patch/postgres_spec.rb | 44 ++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/spec/patch/postgres_spec.rb b/spec/patch/postgres_spec.rb index aa927f1..fc81e36 100644 --- a/spec/patch/postgres_spec.rb +++ b/spec/patch/postgres_spec.rb @@ -7,7 +7,7 @@ def new_client(options = {}) password: ENV.fetch('POSTGRES_PASSWORD', nil), host: ENV.fetch('POSTGRES_HOST', nil), port: ENV.fetch('POSTGRES_PORT', nil), - socket: ENV.fetch('POSTGRES_SOCKET', nil), + socket: ENV.fetch('POSTGRES_SOCKET', nil) }.merge(options)) end @@ -29,7 +29,7 @@ def trip_circuit let(:client) { new_client(database: db_name, faulty: { instance: 'faulty' }) } let(:bad_client) { new_client(host: '127.0.0.1', port: 9999, faulty: { instance: 'faulty' }) } let(:bad_unpatched_client) { new_client(host: '127.0.0.1', port: 9999) } - let(:faulty) { Fai;ty/mew(listeners: [], circuit_defaultts: { sample_threshold: 2 }) } + let(:faulty) { Faulty.new(listeners: [], circuit_defaultts: { sample_threshold: 2 }) } before do new_client.exec("CREATE DATABASE #{db_name}") @@ -40,7 +40,7 @@ def trip_circuit end it 'captures connection error' do - expect {bad_client.query('SELECT 1 FROM dual')}.to raise_error do |error| + expect { bad_client.query('SELECT 1 FROM dual') }.to raise_error do |error| expect(error).to be_a(Faulty::Patch::PG::ConnectionError) expect(error.cause).to be_a(PG::Error::ConnectionBad) end @@ -60,34 +60,34 @@ def trip_circuit it 'successfully executes query' do create_table(client, 'test') client.query('INSERT INTO test VALUES(1)') - expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1'}]) + expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1' }]) expect(faulty.circuit('postgres').status.failure_rate).to eq(0) end it 'prevents additional queries when tripped' do - trip_circuit - expect { client.query('SELECT 1 FROM dual') }.to raise_error(Faulty::Patch::PG::ConnectionError) + trip_circuit + expect { client.query('SELECT 1 FROM dual') }.to raise_error(Faulty::Patch::PG::ConnectionError) end it 'allows COMMIT when tripped' do - create_table(client, 'test') - client.query('BEGIN') - client.query('INSERT INTO test VALUES(1)') - trip_circuit - expect { client.query('COMMIT') }.to be_nil - expect(client.query('SELECT * FROM test')).to raise_error(Faulty::Patch::PG::ConnectionError) - faulty.circuit('postgres').reset - expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1'}]) + create_table(client, 'test') + client.query('BEGIN') + client.query('INSERT INTO test VALUES(1)') + trip_circuit + expect { client.query('COMMIT') }.to be_nil + expect(client.query('SELECT * FROM test')).to raise_error(Faulty::Patch::PG::ConnectionError) + faulty.circuit('postgres').reset + expect(client.query('SELECT * FROM test').to_a).to eq([{ 'id' => '1' }]) end it 'allows ROLLBACK with a leading comment when tripped' do - create_table(client, 'test') - client.query('BEGIN') - client.query('INSERT INTO test VALUES(1)') - trip_circuit - expect { client.query('/* hi there */ ROLLBACK') }.to be_nil - expect { client.query('SELECT * FROM test') }.to raise_error(Faulty::Patch::PG::ConnectionError) - faulty.circuit('postgres').reset - expect(client.query('SELECT * FROM test').to_a).to eq([]) + create_table(client, 'test') + client.query('BEGIN') + client.query('INSERT INTO test VALUES(1)') + trip_circuit + expect { client.query('/* hi there */ ROLLBACK') }.to be_nil + expect { client.query('SELECT * FROM test') }.to raise_error(Faulty::Patch::PG::ConnectionError) + faulty.circuit('postgres').reset + expect(client.query('SELECT * FROM test').to_a).to eq([]) end end From 811b0c3d189a2caa4f0c6aa6c52613a0ed112efc Mon Sep 17 00:00:00 2001 From: tedaford Date: Wed, 28 Sep 2022 15:39:09 -0400 Subject: [PATCH 7/9] update ci.yml --- .github/workflows/ci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5a71f6..1d758a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,14 @@ jobs: ports: - 9200:9200 options: -e="discovery.type=single-node" --health-cmd="curl http://localhost:9200/_cluster/health" --health-interval=3s --health-timeout=5s --health-retries=20 + postgres: + image: postgres:12.6 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: --health-cmd="pg_isready -U postgres" --health-interval=3s --health-timeout=5s --health-retries=20 steps: - uses: actions/checkout@v3 - uses: ruby/setup-ruby@v1 From 24fd14b076596acffecd32aca9643d8fb95c21dd Mon Sep 17 00:00:00 2001 From: Ted Ford <103532252+tedaford@users.noreply.github.com> Date: Sat, 8 Oct 2022 13:02:26 -0400 Subject: [PATCH 8/9] Update Gemfile (#2) * Update Gemfile --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index 68c219c..f8029cb 100644 --- a/Gemfile +++ b/Gemfile @@ -16,7 +16,9 @@ gem 'byebug', platforms: not_jruby gem 'elasticsearch', '> 7', '< 7.14' gem 'honeybadger', '>= 2.0' gem 'irb', '~> 1.0' +gem 'pg', '~> 1.1' # Minimum of 0.5.0 for specific error classes +gem 'mysql2', '>= 0.5.0', platforms: not_jruby gem 'redcarpet', '~> 3.5', platforms: not_jruby gem 'rspec_junit_formatter', '~> 0.4' gem 'yard', '~> 0.9.25', platforms: not_jruby From 04019669e1dc58d5b874e5c3e32ba73d90f91888 Mon Sep 17 00:00:00 2001 From: Ted Ford <103532252+tedaford@users.noreply.github.com> Date: Sat, 8 Oct 2022 13:04:18 -0400 Subject: [PATCH 9/9] update spec_helper.rb (#3) --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 81ed7af..1783284 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,7 @@ require 'faulty' require 'faulty/patch/redis' require 'faulty/patch/elasticsearch' +require 'faulty/patch/postgres' require 'timecop' require 'redis' require 'json'