diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index 8466b13ff2..86f2c85420 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -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, @@ -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] @@ -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 ] addresses the list of addresses diff --git a/lib/mongo/operation/result.rb b/lib/mongo/operation/result.rb index fb8483108a..86c82e5ede 100644 --- a/lib/mongo/operation/result.rb +++ b/lib/mongo/operation/result.rb @@ -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 diff --git a/lib/mongo/retryable.rb b/lib/mongo/retryable.rb index 3cfa383808..5027ac2b46 100644 --- a/lib/mongo/retryable.rb +++ b/lib/mongo/retryable.rb @@ -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}") diff --git a/lib/mongo/retryable/backpressure.rb b/lib/mongo/retryable/backpressure.rb index c36b4273aa..95b9c4b6ca 100644 --- a/lib/mongo/retryable/backpressure.rb +++ b/lib/mongo/retryable/backpressure.rb @@ -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 diff --git a/lib/mongo/retryable/read_worker.rb b/lib/mongo/retryable/read_worker.rb index 8f5456fde2..ef4e10dd87 100644 --- a/lib/mongo/retryable/read_worker.rb +++ b/lib/mongo/retryable/read_worker.rb @@ -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)') diff --git a/lib/mongo/retryable/retry_policy.rb b/lib/mongo/retryable/retry_policy.rb index a4db6fad03..127ac817eb 100644 --- a/lib/mongo/retryable/retry_policy.rb +++ b/lib/mongo/retryable/retry_policy.rb @@ -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. diff --git a/lib/mongo/retryable/write_worker.rb b/lib/mongo/retryable/write_worker.rb index fed52a23ad..6ac97b1cdf 100644 --- a/lib/mongo/retryable/write_worker.rb +++ b/lib/mongo/retryable/write_worker.rb @@ -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)') @@ -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)') diff --git a/lib/mongo/server/app_metadata.rb b/lib/mongo/server/app_metadata.rb index 8278c7a894..5cfb64b189 100644 --- a/lib/mongo/server/app_metadata.rb +++ b/lib/mongo/server/app_metadata.rb @@ -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 diff --git a/lib/mongo/session.rb b/lib/mongo/session.rb index 6c59cbbf36..1ed626e7fe 100644 --- a/lib/mongo/session.rb +++ b/lib/mongo/session.rb @@ -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 @@ -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') @@ -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 diff --git a/spec/mongo/retryable/backpressure_options_spec.rb b/spec/mongo/retryable/backpressure_options_spec.rb index 93a4a7d6ad..9c6b091494 100644 --- a/spec/mongo/retryable/backpressure_options_spec.rb +++ b/spec/mongo/retryable/backpressure_options_spec.rb @@ -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 diff --git a/spec/mongo/retryable/backpressure_spec.rb b/spec/mongo/retryable/backpressure_spec.rb index c8333058a5..c6df07b747 100644 --- a/spec/mongo/retryable/backpressure_spec.rb +++ b/spec/mongo/retryable/backpressure_spec.rb @@ -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 @@ -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 diff --git a/spec/mongo/retryable/client_backpressure_no_backoff_prose_spec.rb b/spec/mongo/retryable/client_backpressure_no_backoff_prose_spec.rb index 055239f830..1a51b4091f 100644 --- a/spec/mongo/retryable/client_backpressure_no_backoff_prose_spec.rb +++ b/spec/mongo/retryable/client_backpressure_no_backoff_prose_spec.rb @@ -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 @@ -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 diff --git a/spec/mongo/retryable/client_backpressure_prose_spec.rb b/spec/mongo/retryable/client_backpressure_prose_spec.rb index 50e82f3d98..9fe24674d8 100644 --- a/spec/mongo/retryable/client_backpressure_prose_spec.rb +++ b/spec/mongo/retryable/client_backpressure_prose_spec.rb @@ -1,127 +1,191 @@ # frozen_string_literal: true -require 'lite_spec_helper' +require 'spec_helper' # Prose tests from the client-backpressure specification: # specifications/source/client-backpressure/tests/README.md +# +# Test 2 was removed from the specification. describe 'Client Backpressure Prose Tests' do - # Shared helpers ---------------------------------------------------------- - - def make_overload_error(message = 'overloaded') - Mongo::Error::OperationFailure.new( - message, nil, - code: 462, - code_name: 'IngressRequestRateLimitExceeded', - labels: %w[RetryableWriteError SystemOverloadedError RetryableError] - ) - end + require_topology :replica_set + min_server_version '4.4' - let(:cluster) { instance_double(Mongo::Cluster) } - let(:server) { instance_double(Mongo::Server) } - let(:server_selector) { instance_double(Mongo::ServerSelector::Primary) } + let(:subscriber) { Mrss::EventSubscriber.new } - let(:session) do - instance_double(Mongo::Session, retry_reads?: true, in_transaction?: false) + let(:client) do + authorized_client.with(retry_reads: true, retry_writes: true).tap do |client| + client.subscribe(Mongo::Monitoring::COMMAND, subscriber) + end end - let(:context) do - instance_double( - Mongo::Operation::Context, - remaining_timeout_sec: nil, csot?: false, deadline: nil - ).tap { |ctx| allow(ctx).to receive(:check_timeout!) } - end + let(:admin_client) { authorized_client.use(:admin) } - shared_context 'with read worker' do - let(:retry_policy) { Mongo::Retryable::RetryPolicy.new } + let(:collection) { client['client-backpressure-prose-test'] } - let(:client) do - instance_double(Mongo::Client).tap do |c| - allow(c).to receive(:retry_policy).and_return(retry_policy) - allow(c).to receive(:cluster).and_return(cluster) - end - end + # Fail every command in commands with an overload error, that is, one + # labeled both SystemOverloadedError and RetryableError. + def set_overload_fail_point(commands, error_code) + admin_client.command( + configureFailPoint: 'failCommand', + mode: 'alwaysOn', + data: { + failCommands: commands, + errorCode: error_code, + errorLabels: %w[SystemOverloadedError RetryableError] + } + ) + end - let(:retryable) do - instance_double(Mongo::Collection, client: client, cluster: cluster).tap do |r| - allow(r).to receive(:select_server).and_return(server) - end + # Duration of an insert that fails with an overload error on every + # attempt, with the random number generator used for jitter pinned to + # the given value. + def failing_insert_duration(jitter) + allow(client.retry_policy).to receive(:rand).and_return(jitter) + start = Mongo::Utils.monotonic_time + error = begin + collection.insert_one(a: 1) + nil + rescue Mongo::Error::OperationFailure => e + e end + elapsed = Mongo::Utils.monotonic_time - start + expect(error).to be_a(Mongo::Error::OperationFailure) + yield(error) if block_given? + elapsed + end - let(:worker) { Mongo::Retryable::ReadWorker.new(retryable) } + def started_events(command_name) + subscriber.started_events.select { |event| event.command_name == command_name } + end - before { allow(worker).to receive(:sleep) } + after do + admin_client.command(configureFailPoint: 'failCommand', mode: 'off') + rescue Mongo::Error + # Ignore cleanup failures. end # ------------------------------------------------------------------------- # Test 1: Operation Retry Uses Exponential Backoff # ------------------------------------------------------------------------- describe 'Test 1: operation retry uses exponential backoff' do - include_context 'with read worker' + it 'waits between retries when jitter is 1 but not when jitter is 0' do + # Step 3.2: fail every insert with an overload error. + set_overload_fail_point(%w[insert], 2) + + # Steps 3.1 and 3.3: a jitter of 0 effectively disables backoff. + no_backoff = failing_insert_duration(0.0) - let(:sleep_args) { [] } + # Steps 3.4 and 3.5: a jitter of 1 gives the full backoff. + with_backoff = failing_insert_duration(1.0) - before do - allow(worker).to receive(:sleep) { |d| sleep_args << d } + # Step 3.6: the sum of the two backoffs is 0.3 seconds. The + # 0.6-second window accounts for variance between the two runs. + expect((with_backoff - (no_backoff + 0.6)).abs).to be < 0.6 end + end - def total_sleep_with_jitter(jitter_value) - allow(retry_policy).to receive(:backoff_delay) { |attempt| - Mongo::Retryable::Backpressure.backoff_delay(attempt, jitter: jitter_value) - } - sleep_args.clear - begin - worker.read_with_retry(session, server_selector, context) { |_s, _r| raise make_overload_error } - rescue Mongo::Error::OperationFailure - # expected + # ------------------------------------------------------------------------- + # Test 3: Overload Errors are Retried a Maximum of MAX_RETRIES times + # ------------------------------------------------------------------------- + describe 'Test 3: overload errors are retried a maximum of MAX_RETRIES times' do + it 'sends MAX_RETRIES + 1 find commands' do + # MAX_RETRIES is 2 in the specification. + expect(client.retry_policy.max_retries).to eq(2) + + # Step 3: fail every find with an overload error. + set_overload_fail_point(%w[find], 462) # IngressRequestRateLimitExceeded + subscriber.clear_events! + + # Step 4: perform a find that fails. + error = begin + collection.find.first + nil + rescue Mongo::Error::OperationFailure => e + e end - sleep_args.sum - end - it 'with jitter=1 the backoff sum is approximately 0.3s' do - no_backoff = total_sleep_with_jitter(0.0) - with_backoff = total_sleep_with_jitter(1.0) - # Sum of 2 backoffs is 0.3 seconds (0.1 + 0.2). - expect((with_backoff - (no_backoff + 0.3)).abs).to be < 0.3 + # Step 5: the error carries both labels. + expect(error).to be_a(Mongo::Error::OperationFailure) + expect(error.label?('RetryableError')).to be true + expect(error.label?('SystemOverloadedError')).to be true + + # Step 6: one initial attempt plus MAX_RETRIES retries. + expect(started_events('find').length).to eq(3) end end # ------------------------------------------------------------------------- - # Test 3: Overload Errors are Retried DEFAULT_MAX_RETRIES Times + # Test 4: Overload Errors are Retried a Maximum of maxAdaptiveRetries + # times when configured # ------------------------------------------------------------------------- - describe 'Test 3: overload errors are retried DEFAULT_MAX_RETRIES times' do - include_context 'with read worker' - - it 'attempts the command exactly DEFAULT_MAX_RETRIES + 1 times' do - call_count = 0 - expect do - worker.read_with_retry(session, server_selector, context) do |_s, _r| - call_count += 1 - raise make_overload_error - end - end.to raise_error(Mongo::Error::OperationFailure) - - expect(call_count).to eq(Mongo::Retryable::Backpressure::DEFAULT_MAX_RETRIES + 1) + describe 'Test 4: overload errors are retried a maximum of maxAdaptiveRetries times' do + # Step 1: a client with maxAdaptiveRetries=1. + let(:client) do + authorized_client.with(retry_reads: true, max_adaptive_retries: 1).tap do |client| + client.subscribe(Mongo::Monitoring::COMMAND, subscriber) + end + end + + it 'sends maxAdaptiveRetries + 1 find commands' do + expect(client.retry_policy.max_retries).to eq(1) + + # Step 3: fail every find with an overload error. + set_overload_fail_point(%w[find], 462) # IngressRequestRateLimitExceeded + subscriber.clear_events! + + # Step 4: perform a find that fails. + error = begin + collection.find.first + nil + rescue Mongo::Error::OperationFailure => e + e + end + + # Step 5: the error carries both labels. + expect(error).to be_a(Mongo::Error::OperationFailure) + expect(error.label?('RetryableError')).to be true + expect(error.label?('SystemOverloadedError')).to be true + + # Step 6: one initial attempt plus maxAdaptiveRetries retries. + expect(started_events('find').length).to eq(2) end end # ------------------------------------------------------------------------- - # Test 4: Overload Errors are Retried maxAdaptiveRetries Times When Configured + # Test 5: Overload Errors with baseBackoffMS override base backoff # ------------------------------------------------------------------------- - describe 'Test 4: overload errors are retried maxAdaptiveRetries times when configured' do - include_context 'with read worker' + describe 'Test 5: overload errors with baseBackoffMS override base backoff' do + min_server_version '9.0' + + # Reset the parameter here as well as inline, so a failure part-way + # through the example cannot leave it set on the shared cluster. + after do + admin_client.command('setParameter' => 1, 'externalClientBaseBackoffMS' => 0) + rescue Mongo::Error + # Ignore cleanup failures. + end - let(:retry_policy) { Mongo::Retryable::RetryPolicy.new(max_retries: 1) } + it 'sends baseBackoffMS in the overload error and uses it for backoff' do + # Steps 4 and 5: time an insert that always fails with an overload error. + set_overload_fail_point(%w[insert], 462) + exponential_backoff_time = failing_insert_duration(1.0) + + # Steps 6 and 7: have the server attach baseBackoffMS, then repeat. + admin_client.command('setParameter' => 1, 'externalClientBaseBackoffMS' => 50) + with_base_backoff_ms_time = failing_insert_duration(1.0) do |err| + # Step 8: the driver parsed the field the server attached. + expect(err.result.base_backoff_ms).to eq(50) + end - it 'attempts the command exactly maxAdaptiveRetries + 1 times' do - call_count = 0 - expect do - worker.read_with_retry(session, server_selector, context) do |_s, _r| - call_count += 1 - raise make_overload_error - end - end.to raise_error(Mongo::Error::OperationFailure) + # Step 9: disable baseBackoffMS on overload errors. + admin_client.command('setParameter' => 1, 'externalClientBaseBackoffMS' => 0) - expect(call_count).to eq(2) + # Step 10: a run can never be faster than the sum of its backoffs. With + # jitter pinned to 1 the default backoffs are 0.2 + 0.4 = 0.6s and the + # baseBackoffMS=50 backoffs are 0.1 + 0.2 = 0.3s. + expect(exponential_backoff_time).to be >= 0.6 + expect(with_base_backoff_ms_time).to be >= 0.3 + expect(with_base_backoff_ms_time).to be < 0.6 end end end diff --git a/spec/mongo/server/app_metadata_backpressure_spec.rb b/spec/mongo/server/app_metadata_backpressure_spec.rb index 063d782736..189ecd5391 100644 --- a/spec/mongo/server/app_metadata_backpressure_spec.rb +++ b/spec/mongo/server/app_metadata_backpressure_spec.rb @@ -6,7 +6,7 @@ describe '#client_document' do it 'includes backpressure: true' do metadata = described_class.new - expect(metadata.client_document[:backpressure]).to be true + expect(metadata.client_document[:backpressure]).to be '2' end end end diff --git a/spec/mongo/session/with_transaction_overload_spec.rb b/spec/mongo/session/with_transaction_overload_spec.rb index 99f4a54ec4..9b11bd54b6 100644 --- a/spec/mongo/session/with_transaction_overload_spec.rb +++ b/spec/mongo/session/with_transaction_overload_spec.rb @@ -79,7 +79,9 @@ def make_commit_transient_overload_error context 'when callback raises TransientTransactionError with SystemOverloadedError' do it 'uses the new overload backoff' do call_count = 0 - expect(session).to receive(:sleep).with(0.1).once + # jitter is pinned to 1.0 above, so the first overload backoff is + # min(MAX_BACKOFF, BASE_BACKOFF * 2**1) = 0.2s. + expect(session).to receive(:sleep).with(0.2).once session.with_transaction do call_count += 1 @@ -92,7 +94,7 @@ def make_commit_transient_overload_error it 'uses the existing backoff' do call_count = 0 expect(session).to receive(:sleep).once - expect(session).not_to receive(:sleep).with(0.1) + expect(session).not_to receive(:sleep).with(0.2) session.with_transaction do call_count += 1 diff --git a/spec/mongo/session_transaction_prose_spec.rb b/spec/mongo/session_transaction_prose_spec.rb index f1c81e60ce..f75f4ec71c 100644 --- a/spec/mongo/session_transaction_prose_spec.rb +++ b/spec/mongo/session_transaction_prose_spec.rb @@ -45,9 +45,9 @@ end # With jitter=0 all requested sleeps are zero; with jitter=1 they sum to - # approximately 1.8 seconds (sum of 13 exponential backoffs, per spec). + # approximately 2.3 seconds (sum of 13 exponential backoffs, per spec). expect(no_backoff_sleeps.sum).to eq(0) - expect(with_backoff_sleeps.sum).to be_within(0.05).of(1.8) + expect(with_backoff_sleeps.sum).to be_within(0.05).of(2.3) end private