Skip to content

Commit

Permalink
Adopts blocklist/allowlist as suitable alternatives for the harmful t…
Browse files Browse the repository at this point in the history
…erms blacklist/whitelist
  • Loading branch information
rthbound committed May 1, 2021
1 parent e45ba09 commit 1434c0a
Show file tree
Hide file tree
Showing 22 changed files with 145 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ErrorHandler < ::Makara::ErrorHandler
HARSH_ERRORS = [
'ActiveRecord::RecordNotUnique',
'ActiveRecord::InvalidForeignKey',
'Makara::Errors::BlacklistConnection'
'Makara::Errors::BlocklistConnection'
].map(&:freeze).freeze

CONNECTION_MATCHERS = [
Expand Down
6 changes: 3 additions & 3 deletions lib/makara.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ module Makara

module Errors
autoload :MakaraError, 'makara/errors/makara_error'
autoload :AllConnectionsBlacklisted, 'makara/errors/all_connections_blacklisted'
autoload :BlacklistConnection, 'makara/errors/blacklist_connection'
autoload :AllConnectionsBlocklisted, 'makara/errors/all_connections_blocklisted'
autoload :BlocklistConnection, 'makara/errors/blocklist_connection'
autoload :NoConnectionsAvailable, 'makara/errors/no_connections_available'
autoload :BlacklistedWhileInTransaction, 'makara/errors/blacklisted_while_in_transaction'
autoload :BlocklistedWhileInTransaction, 'makara/errors/blocklisted_while_in_transaction'
autoload :InvalidShard, 'makara/errors/invalid_shard'
end

Expand Down
4 changes: 2 additions & 2 deletions lib/makara/config_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# another: 'top level variable'
# makara:
# primary_ttl: 3
# blacklist_duration: 20
# blocklist_duration: 20
# connections:
# - role: 'master' # Deprecated in favor of 'primary'
# - role: 'primary'
Expand All @@ -23,7 +23,7 @@ module Makara
class ConfigParser
DEFAULTS = {
primary_ttl: 5,
blacklist_duration: 30,
blocklist_duration: 30,
sticky: true
}

Expand Down
30 changes: 15 additions & 15 deletions lib/makara/connection_wrapper.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'active_support/core_ext/hash/keys'

# Makara::ConnectionWrapper wraps the instance of an underlying connection.
# The wrapper provides methods for tracking blacklisting and individual makara configurations.
# The wrapper provides methods for tracking blocklisting and individual makara configurations.
# Upon creation, the wrapper defines methods in the underlying object giving it access to the
# Makara::Proxy.

Expand All @@ -18,7 +18,7 @@ def initialize(proxy, connection, config)
@proxy = proxy

if connection.nil?
_makara_blacklist!
_makara_blocklist!
else
_makara_decorate_connection(connection)
end
Expand All @@ -38,25 +38,25 @@ def _makara_shard_id
@config[:shard_id]
end

# has this node been blacklisted?
def _makara_blacklisted?
@blacklisted_until.present? && @blacklisted_until.to_i > Time.now.to_i
# has this node been blocklisted?
def _makara_blocklisted?
@blocklisted_until.present? && @blocklisted_until.to_i > Time.now.to_i
end

def _makara_in_transaction?
@connection && @connection.open_transactions > 0
end

# blacklist this node for @config[:blacklist_duration] seconds
def _makara_blacklist!
# blocklist this node for @config[:blocklist_duration] seconds
def _makara_blocklist!
@connection.disconnect! if @connection
@connection = nil
@blacklisted_until = Time.now.to_i + @config[:blacklist_duration] unless @config[:disable_blacklist]
@blocklisted_until = Time.now.to_i + @config[:blocklist_duration] unless @config[:disable_blocklist]
end

# release the blacklist
def _makara_whitelist!
@blacklisted_until = nil
# release the blocklist
def _makara_allowlist!
@blocklisted_until = nil
end

# custom error messages
Expand All @@ -66,7 +66,7 @@ def _makara_custom_error_matchers

def _makara_connected?
_makara_connection.present?
rescue Makara::Errors::BlacklistConnection
rescue Makara::Errors::BlocklistConnection
false
end

Expand All @@ -75,13 +75,13 @@ def _makara_connection

if current
current
else # blacklisted connection or initial error
else # blocklisted connection or initial error
new_connection = @proxy.graceful_connection_for(@config)

# Already wrapped because of initial failure
if new_connection.is_a?(Makara::ConnectionWrapper)
_makara_blacklist!
raise Makara::Errors::BlacklistConnection.new(self, new_connection.initial_error)
_makara_blocklist!
raise Makara::Errors::BlocklistConnection.new(self, new_connection.initial_error)
else
@connection = new_connection
_makara_decorate_connection(new_connection)
Expand Down
2 changes: 1 addition & 1 deletion lib/makara/error_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def handle(connection)
protected

def gracefully(connection, e)
err = Makara::Errors::BlacklistConnection.new(connection, e)
err = Makara::Errors::BlocklistConnection.new(connection, e)
::Makara::Logging::Logger.log("Gracefully handling: #{err}")
raise err
end
Expand Down
4 changes: 2 additions & 2 deletions lib/makara/errors/all_connections_blacklisted.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
module Makara
module Errors
class AllConnectionsBlacklisted < MakaraError
class AllConnectionsBlocklisted < MakaraError
def initialize(pool, errors)
errors = [*errors]
messages = errors.empty? ? 'No error details' : errors.map(&:message).join(' -> ')
super "[Makara/#{pool.role}] All connections are blacklisted -> " + messages
super "[Makara/#{pool.role}] All connections are blocklisted -> " + messages
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/makara/errors/blacklist_connection.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Makara
module Errors
class BlacklistConnection < MakaraError
class BlocklistConnection < MakaraError
attr_reader :original_error

def initialize(connection, error)
Expand Down
4 changes: 2 additions & 2 deletions lib/makara/errors/blacklisted_while_in_transaction.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
module Makara
module Errors
class BlacklistedWhileInTransaction < MakaraError
class BlocklistedWhileInTransaction < MakaraError
attr_reader :role

def initialize(role)
@role = role
super "[Makara] Blacklisted while in transaction in the #{role} pool"
super "[Makara] Blocklisted while in transaction in the #{role} pool"
end
end
end
Expand Down
44 changes: 22 additions & 22 deletions lib/makara/pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Pool
# there are cases when we understand the pool is busted and we essentially want to skip
# all execution
attr_accessor :disabled
attr_reader :blacklist_errors
attr_reader :blocklist_errors
attr_reader :role
attr_reader :connections
attr_reader :strategy
Expand All @@ -20,7 +20,7 @@ def initialize(role, proxy)
@role = role == "master" ? "primary" : role
@proxy = proxy
@connections = []
@blacklist_errors = []
@blocklist_errors = []
@disabled = false
if proxy.shard_aware_for(role)
@strategy = Makara::Strategies::ShardAware.new(self)
Expand All @@ -31,9 +31,9 @@ def initialize(role, proxy)
end
end

def completely_blacklisted?
def completely_blocklisted?
@connections.each do |connection|
return false unless connection._makara_blacklisted?
return false unless connection._makara_blocklisted?
end
true
end
Expand Down Expand Up @@ -66,7 +66,7 @@ def send_to_all(method, *args, &block)
errors = []

@connections.each do |con|
next if con._makara_blacklisted?
next if con._makara_blocklisted?

begin
ret = @proxy.error_handler.handle(con) do
Expand All @@ -78,15 +78,15 @@ def send_to_all(method, *args, &block)
end

one_worked = true
rescue Makara::Errors::BlacklistConnection => e
rescue Makara::Errors::BlocklistConnection => e
errors.insert(0, e)
con._makara_blacklist!
con._makara_blocklist!
end
end

if !one_worked
if connection_made?
raise Makara::Errors::AllConnectionsBlacklisted.new(self, errors)
raise Makara::Errors::AllConnectionsBlocklisted.new(self, errors)
else
raise Makara::Errors::NoConnectionsAvailable.new(@role) unless @disabled
end
Expand All @@ -95,46 +95,46 @@ def send_to_all(method, *args, &block)
ret
end

# Provide a connection that is not blacklisted and connected. Handle any errors
# Provide a connection that is not blocklisted and connected. Handle any errors
# that may occur within the block.
def provide
attempt = 0
begin
provided_connection = self.next

# nil implies that it's blacklisted
# nil implies that it's blocklisted
if provided_connection

value = @proxy.error_handler.handle(provided_connection) do
yield provided_connection
end

@blacklist_errors = []
@blocklist_errors = []

value

# if we've made any connections within this pool, we should report the blackout.
elsif connection_made?
err = Makara::Errors::AllConnectionsBlacklisted.new(self, @blacklist_errors)
@blacklist_errors = []
err = Makara::Errors::AllConnectionsBlocklisted.new(self, @blocklist_errors)
@blocklist_errors = []
raise err
else
raise Makara::Errors::NoConnectionsAvailable.new(@role) unless @disabled
end

# when a connection causes a blacklist error within the provided block, we blacklist it then retry
rescue Makara::Errors::BlacklistConnection => e
@blacklist_errors.insert(0, e)
# when a connection causes a blocklist error within the provided block, we blocklist it then retry
rescue Makara::Errors::BlocklistConnection => e
@blocklist_errors.insert(0, e)
in_transaction = self.role == "primary" && provided_connection._makara_in_transaction?
provided_connection._makara_blacklist!
raise Makara::Errors::BlacklistedWhileInTransaction.new(@role) if in_transaction
provided_connection._makara_blocklist!
raise Makara::Errors::BlocklistedWhileInTransaction.new(@role) if in_transaction

attempt += 1
if attempt < @connections.length
retry
elsif connection_made?
err = Makara::Errors::AllConnectionsBlacklisted.new(self, @blacklist_errors)
@blacklist_errors = []
err = Makara::Errors::AllConnectionsBlocklisted.new(self, @blocklist_errors)
@blocklist_errors = []
raise err
else
raise Makara::Errors::NoConnectionsAvailable.new(@role) unless @disabled
Expand All @@ -149,9 +149,9 @@ def connection_made?
@connections.any?(&:_makara_connected?)
end

# Get the next non-blacklisted connection. If the proxy is setup
# Get the next non-blocklisted connection. If the proxy is setup
# to be sticky, provide back the current connection assuming it is
# not blacklisted.
# not blocklisted.
def next
if @proxy.sticky && (curr = @strategy.current)
curr
Expand Down
16 changes: 8 additions & 8 deletions lib/makara/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,14 @@ def graceful_connection_for(config)
@error_handler.handle(fake_wrapper) do
connection_for(config)
end
rescue Makara::Errors::BlacklistConnection => e
rescue Makara::Errors::BlocklistConnection => e
fake_wrapper.initial_error = e.original_error
fake_wrapper
end

def disconnect!
send_to_all(:disconnect!)
rescue ::Makara::Errors::AllConnectionsBlacklisted, ::Makara::Errors::NoConnectionsAvailable
rescue ::Makara::Errors::AllConnectionsBlocklisted, ::Makara::Errors::NoConnectionsAvailable
# all connections are already down, nothing to do here
end

Expand All @@ -186,7 +186,7 @@ def any_connection
yield con
end
end
rescue ::Makara::Errors::AllConnectionsBlacklisted, ::Makara::Errors::NoConnectionsAvailable
rescue ::Makara::Errors::AllConnectionsBlocklisted, ::Makara::Errors::NoConnectionsAvailable
begin
@primary_pool.disabled = true
@replica_pool.provide do |con|
Expand Down Expand Up @@ -215,13 +215,13 @@ def appropriate_pool(method_name, args)
# for testing purposes
pool = _appropriate_pool(method_name, args)
yield pool
rescue ::Makara::Errors::AllConnectionsBlacklisted, ::Makara::Errors::NoConnectionsAvailable => e
rescue ::Makara::Errors::AllConnectionsBlocklisted, ::Makara::Errors::NoConnectionsAvailable => e
if pool == @primary_pool
@primary_pool.connections.each(&:_makara_whitelist!)
@replica_pool.connections.each(&:_makara_whitelist!)
@primary_pool.connections.each(&:_makara_allowlist!)
@replica_pool.connections.each(&:_makara_allowlist!)
Kernel.raise e
else
@primary_pool.blacklist_errors << e
@primary_pool.blocklist_errors << e
retry
end
end
Expand All @@ -240,7 +240,7 @@ def _appropriate_pool(method_name, args)
@primary_pool

# all replicas are down (or empty)
elsif @replica_pool.completely_blacklisted?
elsif @replica_pool.completely_blocklisted?
stick_to_primary(method_name, args)
@primary_pool

Expand Down
4 changes: 2 additions & 2 deletions lib/makara/strategies/priority_failover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ def next
nil
end

# return the connection if it's not blacklisted
# return the connection if it's not blocklisted
# otherwise return nil
# optionally, store the position and context we're returning
def safe_value(idx)
con = @weighted_connections[idx]
return nil unless con
return nil if con._makara_blacklisted?
return nil if con._makara_blocklisted?

con
end
Expand Down
4 changes: 2 additions & 2 deletions lib/makara/strategies/round_robin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ def next_index(idx)
idx
end

# return the connection if it's not blacklisted
# return the connection if it's not blocklisted
# otherwise return nil
# optionally, store the position and context we're returning
def safe_value(idx, stick = false)
con = @weighted_connections[idx]
return nil unless con
return nil if con._makara_blacklisted?
return nil if con._makara_blocklisted?

if stick
@current_idx = idx
Expand Down
Loading

0 comments on commit 1434c0a

Please sign in to comment.