From 88d5b53d8978f1a08c51af509a795aaafd079b40 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 27 Jun 2019 12:20:30 -0400 Subject: [PATCH 1/6] Disable monitoring io when it is not needed --- spec/mongo/collection_spec.rb | 37 ++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/spec/mongo/collection_spec.rb b/spec/mongo/collection_spec.rb index 89024b7a59..a416a0917b 100644 --- a/spec/mongo/collection_spec.rb +++ b/spec/mongo/collection_spec.rb @@ -101,7 +101,8 @@ describe '#with' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge(monitoring_io: false)) end let(:database) do @@ -133,7 +134,8 @@ context 'when the client has a server selection timeout setting' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(server_selection_timeout: 2)) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge(server_selection_timeout: 2, monitoring_io: false)) end it 'passes the the server_selection_timeout to the cluster' do @@ -144,7 +146,11 @@ context 'when the client has a read preference set' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(read: { mode: :primary_preferred })) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge( + read: { mode: :primary_preferred }, + monitoring_io: false, + )) end it 'sets the new read options on the new collection' do @@ -156,7 +162,12 @@ context 'when the client has a read preference and server selection timeout set' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(read: { mode: :primary_preferred }, server_selection_timeout: 2)) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge( + read: { mode: :primary_preferred }, + server_selection_timeout: 2, + monitoring_io: false + )) end it 'sets the new read options on the new collection' do @@ -186,7 +197,11 @@ context 'when the client has a write concern set' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(write: INVALID_WRITE_CONCERN)) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge( + write: INVALID_WRITE_CONCERN, + monitoring_io: false, + )) end it 'sets the new write options on the new collection' do @@ -219,7 +234,11 @@ context 'when the client has a server selection timeout setting' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(server_selection_timeout: 2)) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge( + server_selection_timeout: 2, + monitoring_io: false, + )) end it 'passes the server_selection_timeout setting to the cluster' do @@ -230,7 +249,11 @@ context 'when the client has a read preference set' do let(:client) do - new_local_client(SpecConfig.instance.addresses, SpecConfig.instance.test_options.merge(read: { mode: :primary_preferred })) + new_local_client(SpecConfig.instance.addresses, + SpecConfig.instance.test_options.merge( + read: { mode: :primary_preferred }, + monitoring_io: false, + )) end it 'sets the new read options on the new collection' do From 2f44f5b0ab246eb9dbb8f0d1868d9b7149845cbe Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 29 Jun 2019 02:02:20 -0400 Subject: [PATCH 2/6] Fix read concern test --- .evergreen/config.yml | 2 +- spec/integration/read_concern_spec.rb | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 89b29a6a14..998fcb751c 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -781,7 +781,7 @@ buildvariants: matrix_spec: lint: on ruby: "ruby-2.6" - mongodb-version: ["4.0"] + mongodb-version: ["4.0", "4.2"] topology: [replica-set, sharded-cluster] display_name: "${mongodb-version} ${topology} ${lint} ${ruby}" run_on: diff --git a/spec/integration/read_concern_spec.rb b/spec/integration/read_concern_spec.rb index 3aea7f5322..56f06a0c9c 100644 --- a/spec/integration/read_concern_spec.rb +++ b/spec/integration/read_concern_spec.rb @@ -8,6 +8,10 @@ end let(:specified_read_concern) do + { :level => :local } + end + + let(:expected_read_concern) do { 'level' => 'local' } end @@ -19,7 +23,7 @@ shared_examples_for 'a read concern is specified' do it 'sends a read concern to the server' do - expect(sent_read_concern).to eq(specified_read_concern) + expect(sent_read_concern).to eq(expected_read_concern) end end From 40ad24dff744c3038158eb2cc0aef443d6977fbf Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 29 Jun 2019 02:10:07 -0400 Subject: [PATCH 3/6] Hack the test to make read concern a symbol --- spec/support/transactions/operation.rb | 6 ++++++ spec/support/transactions/test.rb | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/spec/support/transactions/operation.rb b/spec/support/transactions/operation.rb index 113e123fee..5cca46db79 100644 --- a/spec/support/transactions/operation.rb +++ b/spec/support/transactions/operation.rb @@ -139,6 +139,12 @@ def with_transaction(session, context, collection) if arguments['options'] options = Utils.snakeize_hash(arguments['options']) + if options[:read_concern] + options[:read_concern] = Options::Mapper.transform_keys_to_symbols(options[:read_concern]) + if options[:read_concern][:level] == 'majority' + options[:read_concern][:level] = :majority + end + end else options = nil end diff --git a/spec/support/transactions/test.rb b/spec/support/transactions/test.rb index c6f960f4b2..ec12ac6483 100644 --- a/spec/support/transactions/test.rb +++ b/spec/support/transactions/test.rb @@ -54,6 +54,12 @@ def initialize(data, test, spec) @data = data @description = test['description'] @client_options = Utils.convert_client_options(test['clientOptions'] || {}) + if @client_options[:read_concern] + @client_options[:read_concern] = Options::Mapper.transform_keys_to_symbols(@client_options[:read_concern]) + if @client_options[:read_concern][:level].is_a?(String) + @client_options[:read_concern][:level] = @client_options[:read_concern][:level].to_sym + end + end @session_options = Utils.snakeize_hash(test['sessionOptions'] || {}) @fail_point = test['failPoint'] @operations = test['operations'] From e61af45bd0b7bb467c0dae65a7e04317e3b017a2 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 29 Jun 2019 02:20:37 -0400 Subject: [PATCH 4/6] Fix read concern in URI parsing --- lib/mongo/uri.rb | 2 +- spec/mongo/uri_option_parsing_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mongo/uri.rb b/lib/mongo/uri.rb index 4c375d9152..3102a094a3 100644 --- a/lib/mongo/uri.rb +++ b/lib/mongo/uri.rb @@ -554,7 +554,7 @@ def self.uri_option(uri_key, name, extra = {}) # Client Options uri_option 'appname', :app_name uri_option 'compressors', :compressors, :type => :array - uri_option 'readconcernlevel', :level, group: :read_concern + uri_option 'readconcernlevel', :level, group: :read_concern, type: :symbol uri_option 'retryreads', :retry_reads, :type => :retry_reads uri_option 'retrywrites', :retry_writes, :type => :retry_writes uri_option 'zlibcompressionlevel', :zlib_compression_level, :type => :zlib_compression_level diff --git a/spec/mongo/uri_option_parsing_spec.rb b/spec/mongo/uri_option_parsing_spec.rb index 115ee185e5..c414f6e6b0 100644 --- a/spec/mongo/uri_option_parsing_spec.rb +++ b/spec/mongo/uri_option_parsing_spec.rb @@ -319,8 +319,8 @@ it_behaves_like 'parses successfully' - it 'is a string' do - expect(uri.uri_options[:read_concern]).to eq(BSON::Document.new(level: 'snapshot')) + it 'is a symbol' do + expect(uri.uri_options[:read_concern]).to eq(BSON::Document.new(level: :snapshot)) end end From 86dad135d1d7ad2989bc0c16b274f3c1cf494c6e Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 29 Jun 2019 02:27:53 -0400 Subject: [PATCH 5/6] Fix URI spec tests --- lib/mongo/lint.rb | 11 +++++++++-- spec/support/connection_string.rb | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/mongo/lint.rb b/lib/mongo/lint.rb index e96ebc2047..21695c7325 100644 --- a/lib/mongo/lint.rb +++ b/lib/mongo/lint.rb @@ -67,11 +67,18 @@ def validate_read_concern_option(read_concern) end return if read_concern.empty? keys = read_concern.keys - if keys != [:level] + if read_concern.is_a?(BSON::Document) + # Permits indifferent access + allowed_keys = ['level'] + else + # Does not permit indifferent access + allowed_keys = [:level] + end + if keys != allowed_keys raise Error::LintError, "Read concern has invalid keys: #{keys.inspect}" end level = read_concern[:level] - return if [:local, :majority, :snapshot].include?(level) + return if [:local, :available, :majority, :linearizable, :snapshot].include?(level) raise Error::LintError, "Read concern level is invalid: #{level.inspect}" end module_function :validate_read_concern_option diff --git a/spec/support/connection_string.rb b/spec/support/connection_string.rb index cd76cf5cfe..c674daf690 100644 --- a/spec/support/connection_string.rb +++ b/spec/support/connection_string.rb @@ -123,6 +123,7 @@ def tests end class Test + include RSpec::Core::Pending attr_reader :description attr_reader :uri_string @@ -153,6 +154,10 @@ def options def client @client ||= ClientRegistry.instance.new_local_client(@spec['uri'], monitoring_io: false) + rescue Mongo::Error::LintError => e + if e.message =~ /arbitraryButStillValid/ + skip 'Test uses a read concern that fails linter' + end end def uri From 7d43a924a558057e44523ab5d34f9e9b6bd4a5ae Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 29 Jun 2019 02:45:38 -0400 Subject: [PATCH 6/6] Convert read concern values to strings everywhere --- lib/mongo/collection/view/builder/aggregation.rb | 5 ++++- lib/mongo/collection/view/builder/find_command.rb | 5 ++++- lib/mongo/collection/view/builder/map_reduce.rb | 5 ++++- lib/mongo/collection/view/readable.rb | 15 ++++++++++++--- lib/mongo/operation/parallel_scan/command.rb | 5 ++++- lib/mongo/operation/parallel_scan/op_msg.rb | 5 ++++- lib/mongo/operation/shared/sessions_supported.rb | 4 +++- lib/mongo/session.rb | 6 +++--- 8 files changed, 38 insertions(+), 12 deletions(-) diff --git a/lib/mongo/collection/view/builder/aggregation.rb b/lib/mongo/collection/view/builder/aggregation.rb index 8248cf07da..7b395bd133 100644 --- a/lib/mongo/collection/view/builder/aggregation.rb +++ b/lib/mongo/collection/view/builder/aggregation.rb @@ -93,7 +93,10 @@ def write? def aggregation_command command = BSON::Document.new(:aggregate => collection.name, :pipeline => pipeline) command[:cursor] = cursor if cursor - command[:readConcern] = collection.read_concern if collection.read_concern + if collection.read_concern + command[:readConcern] = Options::Mapper.transform_values_to_strings( + collection.read_concern) + end command.merge!(Options::Mapper.transform_documents(options, MAPPINGS)) command end diff --git a/lib/mongo/collection/view/builder/find_command.rb b/lib/mongo/collection/view/builder/find_command.rb index fc6e72138e..9b44c7ca91 100644 --- a/lib/mongo/collection/view/builder/find_command.rb +++ b/lib/mongo/collection/view/builder/find_command.rb @@ -96,7 +96,10 @@ def specification def find_command document = BSON::Document.new('find' => collection.name, 'filter' => filter) - document[:readConcern] = collection.read_concern if collection.read_concern + if collection.read_concern + document[:readConcern] = Options::Mapper.transform_values_to_strings( + collection.read_concern) + end command = Options::Mapper.transform_documents(convert_flags(options), MAPPINGS, document) convert_limit_and_batch_size(command) command diff --git a/lib/mongo/collection/view/builder/map_reduce.rb b/lib/mongo/collection/view/builder/map_reduce.rb index b72ef97115..b8339201da 100644 --- a/lib/mongo/collection/view/builder/map_reduce.rb +++ b/lib/mongo/collection/view/builder/map_reduce.rb @@ -139,7 +139,10 @@ def map_reduce_command :query => filter, :out => { inline: 1 } ) - command[:readConcern] = collection.read_concern if collection.read_concern + if collection.read_concern + command[:readConcern] = Options::Mapper.transform_values_to_strings( + collection.read_concern) + end command.merge!(view_options) command.merge!(Options::Mapper.transform_documents(options, MAPPINGS)) command diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index 2344abea32..9d55eb0da1 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -138,7 +138,10 @@ def count(opts = {}) cmd[:skip] = opts[:skip] if opts[:skip] cmd[:hint] = opts[:hint] if opts[:hint] cmd[:limit] = opts[:limit] if opts[:limit] - cmd[:readConcern] = read_concern if read_concern + if read_concern + cmd[:readConcern] = Options::Mapper.transform_values_to_strings( + read_concern) + end cmd[:maxTimeMS] = opts[:max_time_ms] if opts[:max_time_ms] Mongo::Lint.validate_underscore_read_preference(opts[:read]) read_pref = opts[:read] || read_preference @@ -205,7 +208,10 @@ def count_documents(opts = {}) def estimated_document_count(opts = {}) cmd = { count: collection.name } cmd[:maxTimeMS] = opts[:max_time_ms] if opts[:max_time_ms] - cmd[:readConcern] = read_concern if read_concern + if read_concern + cmd[:readConcern] = Options::Mapper.transform_values_to_strings( + read_concern) + end Mongo::Lint.validate_underscore_read_preference(opts[:read]) read_pref = opts[:read] || read_preference selector = ServerSelector.get(read_pref || server_selector) @@ -242,7 +248,10 @@ def distinct(field_name, opts = {}) :key => field_name.to_s, :query => filter } cmd[:maxTimeMS] = opts[:max_time_ms] if opts[:max_time_ms] - cmd[:readConcern] = read_concern if read_concern + if read_concern + cmd[:readConcern] = Options::Mapper.transform_values_to_strings( + read_concern) + end Mongo::Lint.validate_underscore_read_preference(opts[:read]) read_pref = opts[:read] || read_preference selector = ServerSelector.get(read_pref || server_selector) diff --git a/lib/mongo/operation/parallel_scan/command.rb b/lib/mongo/operation/parallel_scan/command.rb index 31cf6ea06a..ca26d3e134 100644 --- a/lib/mongo/operation/parallel_scan/command.rb +++ b/lib/mongo/operation/parallel_scan/command.rb @@ -32,7 +32,10 @@ class Command def selector(server) sel = { :parallelCollectionScan => coll_name, :numCursors => cursor_count } - sel[:readConcern] = read_concern if read_concern + if read_concern + sel[:readConcern] = Options::Mapper.transform_values_to_strings( + read_concern) + end sel[:maxTimeMS] = max_time_ms if max_time_ms update_selector_for_read_pref(sel, server) sel diff --git a/lib/mongo/operation/parallel_scan/op_msg.rb b/lib/mongo/operation/parallel_scan/op_msg.rb index d26d4abe85..9bb20675dc 100644 --- a/lib/mongo/operation/parallel_scan/op_msg.rb +++ b/lib/mongo/operation/parallel_scan/op_msg.rb @@ -31,7 +31,10 @@ class OpMsg < OpMsgBase def selector(server) sel = { :parallelCollectionScan => coll_name, :numCursors => cursor_count } sel[:maxTimeMS] = max_time_ms if max_time_ms - sel[:readConcern] = read_concern if read_concern + if read_concern + sel[:readConcern] = Options::Mapper.transform_values_to_strings( + read_concern) + end sel end end diff --git a/lib/mongo/operation/shared/sessions_supported.rb b/lib/mongo/operation/shared/sessions_supported.rb index 6e27c73d4e..c1754bb7f6 100644 --- a/lib/mongo/operation/shared/sessions_supported.rb +++ b/lib/mongo/operation/shared/sessions_supported.rb @@ -62,7 +62,9 @@ def apply_causal_consistency_if_possible(selector, server) if !server.standalone? cc_doc = session.send(:causal_consistency_doc) if cc_doc - selector[:readConcern] = (selector[:readConcern] || read_concern || {}).merge(cc_doc) + rc_doc = (selector[:readConcern] || read_concern || {}).merge(cc_doc) + selector[:readConcern] = Options::Mapper.transform_values_to_strings( + rc_doc) end end end diff --git a/lib/mongo/session.rb b/lib/mongo/session.rb index bf9a213665..13dc663267 100644 --- a/lib/mongo/session.rb +++ b/lib/mongo/session.rb @@ -804,13 +804,13 @@ def add_txn_opts!(command, read) if rc.nil? || rc.empty? c.delete(:readConcern) else - c[:readConcern ] = rc + c[:readConcern ] = Options::Mapper.transform_values_to_strings(rc) end end # We need to send the read concern level as a string rather than a symbol. - if c[:readConcern] && c[:readConcern][:level] - c[:readConcern][:level] = c[:readConcern][:level].to_s + if c[:readConcern] + c[:readConcern] = Options::Mapper.transform_values_to_strings(c[:readConcern]) end # The write concern should be added to any abortTransaction or commitTransaction command.