Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions lib/mongo/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -613,9 +613,7 @@ def initialize(addresses_or_uri, options = nil)
sdam_proc.call(self) if sdam_proc

@connect_lock = Mutex.new
@retry_policy = Retryable::RetryPolicy.new(
max_retries: @options[:max_adaptive_retries] || Retryable::Backpressure::DEFAULT_MAX_RETRIES
)
@retry_policy = build_retry_policy
@connect_lock.synchronize do
@cluster = Cluster.new(
addresses,
Expand Down Expand Up @@ -846,6 +844,12 @@ def update_options(new_options)
options.update(opts)
@options = options.freeze

# The retry policy is built from the options, so a client created by
# #with needs its own policy when the option changed.
if @options[:max_adaptive_retries] != old_options[:max_adaptive_retries]
@retry_policy = build_retry_policy
end

auto_encryption_options_changed =
@options[:auto_encryption_options] != old_options[:auto_encryption_options]

Expand Down Expand Up @@ -1276,6 +1280,16 @@ def tracer

private

# Builds the retry policy for the backpressure retry loops from the
# client's options.
#
# @return [ Retryable::RetryPolicy ] The retry policy.
def build_retry_policy
Retryable::RetryPolicy.new(
max_retries: @options[:max_adaptive_retries] || Retryable::Backpressure::DEFAULT_MAX_RETRIES
)
end

# Attempts to parse the given list of addresses, using the provided options.
#
# @param [ String | Array<String> ] addresses the list of addresses
Expand Down
9 changes: 9 additions & 0 deletions lib/mongo/operation/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,15 @@ def snapshot_timestamp
doc['cursor']&.[]('atClusterTime') || doc['atClusterTime']
end

# Returns the base backoff in milliseconds for a server overload error, if present.
#
# @return [ Integer | nil ] The base backoff in milliseconds.
#
# @api private
def base_backoff_ms
first_document && first_document['baseBackoffMS']
end

private

def operation_failure_class
Expand Down
2 changes: 1 addition & 1 deletion lib/mongo/retryable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def with_overload_retry(context: nil, retry_enabled: true)

error_count += 1
policy = client.retry_policy
delay = policy.backoff_delay(error_count)
delay = policy.backoff_delay(error_count, err: e)
raise e unless policy.should_retry_overload?(error_count, delay, context: context)

Logger.logger.warn("Overload retry due to: #{e.class.name}: #{e.message}")
Expand Down
19 changes: 17 additions & 2 deletions lib/mongo/retryable/backpressure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,24 @@ module Backpressure
# a random value. Can be injected for deterministic testing.
#
# @return [ Float ] The backoff delay in seconds.
def self.backoff_delay(attempt, jitter: rand)
jitter * [ MAX_BACKOFF, BASE_BACKOFF * (2**(attempt - 1)) ].min
def self.backoff_delay(attempt, jitter: rand, err: nil)
jitter * [ MAX_BACKOFF, base_backoff(err) * (2**attempt) ].min
end

def self.base_backoff(err)
return BASE_BACKOFF if err.nil?
return BASE_BACKOFF unless err.respond_to?(:result) && err.result.respond_to?(:base_backoff_ms)

base_backoff_ms = err.result.base_backoff_ms

if base_backoff_ms && base_backoff_ms > 0
err.result.base_backoff_ms / 1000.0
else
BASE_BACKOFF
end
end

private_class_method :base_backoff
end
end
end
2 changes: 1 addition & 1 deletion lib/mongo/retryable/read_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ def retry_read(original_error, session, server_selector, context: nil, failed_se
def overload_read_retry(last_error, session, server_selector, context, failed_server, error_count:)
last_was_overload = true
loop do
delay = last_was_overload ? retry_policy.backoff_delay(error_count) : 0
delay = last_was_overload ? retry_policy.backoff_delay(error_count, err: last_error) : 0
raise last_error unless retry_policy.should_retry_overload?(error_count, delay, context: context)

log_retry(last_error, message: 'Read retry (overload backoff)')
Expand Down
4 changes: 2 additions & 2 deletions lib/mongo/retryable/retry_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ def initialize(max_retries: Backpressure::DEFAULT_MAX_RETRIES)
# @param [ Float ] jitter A random float in [0.0, 1.0).
#
# @return [ Float ] The backoff delay in seconds.
def backoff_delay(attempt, jitter: rand)
Backpressure.backoff_delay(attempt, jitter: jitter)
def backoff_delay(attempt, jitter: rand, err: nil)
Backpressure.backoff_delay(attempt, jitter: jitter, err: err)
end

# Determine whether an overload retry should be attempted.
Expand Down
4 changes: 2 additions & 2 deletions lib/mongo/retryable/write_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def nro_write_with_retry(_write_concern, context:, &block)
unless e.respond_to?(:label?) && e.label?('NoWritesPerformed')
error_to_raise = e
end
delay = retry_policy.backoff_delay(error_count)
delay = retry_policy.backoff_delay(error_count, err: e)
raise error_to_raise unless retry_policy.should_retry_overload?(error_count, delay, context: context)

log_retry(e, message: 'Write retry (overload backoff)')
Expand Down Expand Up @@ -386,7 +386,7 @@ def overload_write_retry(last_error, session, txn_num, context:, failed_server:,
last_was_overload = true

loop do
delay = last_was_overload ? retry_policy.backoff_delay(error_count) : 0
delay = last_was_overload ? retry_policy.backoff_delay(error_count, err: last_error) : 0
raise error_to_raise unless retry_policy.should_retry_overload?(error_count, delay, context: context)

log_retry(last_error, message: 'Write retry (overload backoff)')
Expand Down
2 changes: 1 addition & 1 deletion lib/mongo/server/app_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def client_document
doc[:driver] = driver_doc
doc[:os] = os_doc
doc[:platform] = platform_string
doc[:backpressure] = true
doc[:backpressure] = '2'
env_doc.tap { |env| doc[:env] = env if env }
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/mongo/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ def with_transaction(options = nil)
loop do
if transaction_attempt > 0
if overload_encountered
delay = @client.retry_policy.backoff_delay(overload_error_count)
delay = @client.retry_policy.backoff_delay(overload_error_count, err: last_error)
if backoff_would_exceed_deadline?(deadline, delay)
make_timeout_error_from(last_error, 'CSOT timeout expired waiting to retry withTransaction')
end
Expand Down Expand Up @@ -562,7 +562,7 @@ def with_transaction(options = nil)
end

if overload_encountered
delay = @client.retry_policy.backoff_delay(overload_error_count)
delay = @client.retry_policy.backoff_delay(overload_error_count, err: e)
if backoff_would_exceed_deadline?(deadline, delay)
transaction_in_progress = false
make_timeout_error_from(e, 'CSOT timeout expired during withTransaction commit')
Expand Down Expand Up @@ -1429,7 +1429,7 @@ def deadline_expired?(deadline)
private_constant :BACKOFF_INITIAL, :BACKOFF_MAX

def backoff_seconds_for_retry(transaction_attempt)
exponential = BACKOFF_INITIAL * (1.5**(transaction_attempt - 1))
exponential = BACKOFF_INITIAL * (1.5**transaction_attempt)
Random.rand * [ exponential, BACKOFF_MAX ].min
end

Expand Down
23 changes: 23 additions & 0 deletions spec/mongo/retryable/backpressure_options_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,29 @@
client = new_local_client_nmio([ 'localhost:27017' ], max_adaptive_retries: 4)
expect(client.retry_policy.max_retries).to eq(4)
end

context 'when derived via Client#with' do
let(:client) { new_local_client_nmio([ 'localhost:27017' ], max_adaptive_retries: 4) }

it 'rebuilds the retry policy with the new value' do
expect(client.with(max_adaptive_retries: 1).retry_policy.max_retries).to eq(1)
end

it 'reverts to the default when the option is removed' do
expect(client.with(max_adaptive_retries: nil).retry_policy.max_retries)
.to eq(Mongo::Retryable::Backpressure::DEFAULT_MAX_RETRIES)
end

it 'leaves the original client policy alone' do
client.with(max_adaptive_retries: 1)
expect(client.retry_policy.max_retries).to eq(4)
end

it 'keeps the policy when an unrelated option changes' do
derived = client.with(read: { mode: :secondary })
expect(derived.retry_policy.max_retries).to eq(4)
end
end
end

describe 'enableOverloadRetargeting' do
Expand Down
109 changes: 102 additions & 7 deletions spec/mongo/retryable/backpressure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@
expect(described_class.backoff_delay(5, jitter: 0)).to eq(0)
end

it 'returns exact exponential values when jitter is 1' do
expect(described_class.backoff_delay(1, jitter: 1)).to eq(0.1)
expect(described_class.backoff_delay(2, jitter: 1)).to eq(0.2)
expect(described_class.backoff_delay(3, jitter: 1)).to eq(0.4)
expect(described_class.backoff_delay(4, jitter: 1)).to eq(0.8)
expect(described_class.backoff_delay(5, jitter: 1)).to eq(1.6)
# backoff = jitter * min(MAX_BACKOFF, BASE_BACKOFF * 2**attempt)
it 'returns BASE_BACKOFF * 2**attempt when jitter is 1' do
expect(described_class.backoff_delay(1, jitter: 1)).to eq(0.2)
expect(described_class.backoff_delay(2, jitter: 1)).to eq(0.4)
expect(described_class.backoff_delay(3, jitter: 1)).to eq(0.8)
expect(described_class.backoff_delay(4, jitter: 1)).to eq(1.6)
expect(described_class.backoff_delay(5, jitter: 1)).to eq(3.2)
end

it 'caps at MAX_BACKOFF for large attempt numbers' do
Expand All @@ -39,7 +40,101 @@
100.times do
delay = described_class.backoff_delay(1)
expect(delay).to be >= 0
expect(delay).to be < 0.1
expect(delay).to be < 0.2
end
end

it 'uses the default base backoff when no error is given' do
expect(described_class.backoff_delay(1, jitter: 1, err: nil)).to eq(0.2)
end
end

describe '.backoff_delay with a server-supplied baseBackoffMS' do
let(:reply_document) do
{
'code' => 462,
'codeName' => 'IngressRequestRateLimitExceeded',
'errorLabels' => %w[SystemOverloadedError RetryableError],
}.merge(extra_fields)
end

let(:extra_fields) do
{}
end

# Built by hand rather than by Protocol::Reply::deserialize, so the fields
# need to be set directly.
let(:reply) do
Mongo::Protocol::Reply.new.tap do |r|
r.instance_variable_set(:@documents, [ reply_document ])
r.instance_variable_set(:@flags, [])
end
end

let(:error) do
Mongo::Error::OperationFailure.new(
'overloaded',
Mongo::Operation::Result.new(reply, Mongo::Server::Description.new(''))
)
end

context 'when baseBackoffMS is positive' do
let(:extra_fields) do
{ 'baseBackoffMS' => 50 }
end

it 'uses it in place of BASE_BACKOFF' do
# These are the delays prose test 5 measures: 0.05 * 2 and 0.05 * 4.
expect(described_class.backoff_delay(1, jitter: 1, err: error)).to eq(0.1)
expect(described_class.backoff_delay(2, jitter: 1, err: error)).to eq(0.2)
end

it 'still applies jitter and the MAX_BACKOFF cap' do
expect(described_class.backoff_delay(1, jitter: 0, err: error)).to eq(0)
expect(described_class.backoff_delay(100, jitter: 1, err: error)).to eq(10)
end
end

context 'when baseBackoffMS is absent' do
it 'uses BASE_BACKOFF' do
expect(described_class.backoff_delay(1, jitter: 1, err: error)).to eq(0.2)
end
end

context 'when baseBackoffMS is zero' do
let(:extra_fields) do
{ 'baseBackoffMS' => 0 }
end

# The spec requires the override only when the value is positive.
it 'uses BASE_BACKOFF' do
expect(described_class.backoff_delay(1, jitter: 1, err: error)).to eq(0.2)
end
end

context 'when baseBackoffMS is negative' do
let(:extra_fields) do
{ 'baseBackoffMS' => -50 }
end

it 'uses BASE_BACKOFF' do
expect(described_class.backoff_delay(1, jitter: 1, err: error)).to eq(0.2)
end
end

context 'when the error carries no result' do
# The connection pool labels network errors raised during connection
# establishment with SystemOverloadedError and RetryableError, so an
# error without a result can reach the overload retry loops.
let(:error) do
Mongo::Error::SocketError.new('connection reset').tap do |err|
err.add_label('SystemOverloadedError')
err.add_label('RetryableError')
end
end

it 'uses BASE_BACKOFF' do
expect(described_class.backoff_delay(1, jitter: 1, err: error)).to eq(0.2)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,21 @@

let(:subscriber) { Mrss::EventSubscriber.new }

# The delay of a single backoff at attempt 1 with jitter pinned to 1, per
# jitter * min(MAX_BACKOFF, BASE_BACKOFF * 2**attempt).
let(:one_backoff) do
[
Mongo::Retryable::Backpressure::MAX_BACKOFF,
Mongo::Retryable::Backpressure::BASE_BACKOFF * 2,
].min
end

before do
# Inflate BASE_BACKOFF so any accidental backoff is clearly visible
# through timing. Without backoff the operation completes in
# milliseconds; with backoff it would take at least 5 seconds.
stub_const('Mongo::Retryable::Backpressure::BASE_BACKOFF', 5.0)
# through timing: without backoff the operation completes in
# milliseconds. Jitter is pinned so the timing is deterministic.
stub_const('Mongo::Retryable::Backpressure::BASE_BACKOFF', 0.5)
allow(client.retry_policy).to receive(:rand).and_return(1.0)
end

after do
Expand Down Expand Up @@ -77,11 +87,12 @@
end.to raise_error(Mongo::Error::OperationFailure)
elapsed = Mongo::Utils.monotonic_time - start_time

# With BASE_BACKOFF=5s, correct behavior applies one backoff
# (bounded by BASE_BACKOFF) for the overload error, then retries
# non-overload errors immediately. The elapsed time should stay
# under BASE_BACKOFF plus a small margin for network overhead.
expect(elapsed).to be < Mongo::Retryable::Backpressure::BASE_BACKOFF + 2
# Correct behavior applies exactly one backoff, for the overload error,
# then retries the non-overload errors immediately. Backing off a second
# time would add min(MAX_BACKOFF, BASE_BACKOFF * 2**2), i.e. twice as
# much again, so the upper bound cleanly separates the two.
expect(elapsed).to be >= one_backoff
expect(elapsed).to be < one_backoff * 2
end
end
end
Loading
Loading