From ba80fb6a477c50c97f4542814780eff5b7f8ffc9 Mon Sep 17 00:00:00 2001 From: Danil Nurgaliev Date: Sat, 16 Dec 2023 11:32:47 +0300 Subject: [PATCH 1/3] ES 8.x upgrade --- chewy.gemspec | 5 ++-- docker-compose.yml | 16 +++++++++++ lib/chewy/index/actions.rb | 10 +++---- lib/chewy/index/aliases.rb | 2 +- lib/chewy/index/syncer.rb | 2 +- lib/chewy/search/request.rb | 4 +-- spec/chewy/index/actions_spec.rb | 16 +++++------ spec/chewy/index/import/bulk_builder_spec.rb | 2 +- spec/chewy/index/import_spec.rb | 28 ++++++++++---------- spec/chewy/index/specification_spec.rb | 3 --- spec/chewy/runtime_spec.rb | 4 +-- spec/chewy/search/response_spec.rb | 2 +- spec/chewy_spec.rb | 2 +- 13 files changed, 55 insertions(+), 41 deletions(-) create mode 100644 docker-compose.yml diff --git a/chewy.gemspec b/chewy.gemspec index 866ed3c36..482a4d9a1 100644 --- a/chewy.gemspec +++ b/chewy.gemspec @@ -11,13 +11,14 @@ Gem::Specification.new do |spec| spec.description = 'Chewy provides functionality for Elasticsearch index handling, documents import mappings and chainable query DSL' spec.homepage = 'https://github.com/toptal/chewy' spec.license = 'MIT' + spec.required_ruby_version = '~> 3.0' spec.files = `git ls-files`.split($RS) spec.executables = spec.files.grep(%r{^bin/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_dependency 'activesupport', '>= 5.2' # Remove with major version bump, 8.x - spec.add_dependency 'elasticsearch', '>= 7.12.0', '< 7.14.0' + spec.add_dependency 'activesupport', '>= 6.1' + spec.add_dependency 'elasticsearch', '>= 8.11', '< 9.0' spec.add_dependency 'elasticsearch-dsl' spec.metadata['rubygems_mfa_required'] = 'true' end diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 000000000..41042bf1d --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,16 @@ +version: "3.4" +services: + elasticsearch_test: + image: "elasticsearch:8.11.3" + environment: + - bootstrap.memory_lock=${ES_MEMORY_LOCK:-false} + - "ES_JAVA_OPTS=-Xms${TEST_ES_HEAP_SIZE:-500m} -Xmx${TEST_ES_HEAP_SIZE:-500m}" + - discovery.type=single-node + - xpack.security.enabled=false + - action.destructive_requires_name=false + ports: + - "127.0.0.1:9250:9200" + ulimits: + nofile: + soft: 65536 + hard: 65536 diff --git a/lib/chewy/index/actions.rb b/lib/chewy/index/actions.rb index afc7debcc..a146f47cc 100644 --- a/lib/chewy/index/actions.rb +++ b/lib/chewy/index/actions.rb @@ -32,7 +32,7 @@ def exists? # def create(*args, **kwargs) create!(*args, **kwargs) - rescue Elasticsearch::Transport::Transport::Errors::BadRequest + rescue Elastic::Transport::Transport::Errors::BadRequest false end @@ -83,9 +83,9 @@ def delete(suffix = nil) result = client.indices.delete index: index_names.join(',') Chewy.wait_for_status if result result - # es-ruby >= 1.0.10 handles Elasticsearch::Transport::Transport::Errors::NotFound + # es-ruby >= 1.0.10 handles Elastic::Transport::Transport::Errors::NotFound # by itself, rescue is for previous versions - rescue Elasticsearch::Transport::Transport::Errors::NotFound + rescue Elastic::Transport::Transport::Errors::NotFound false end @@ -99,9 +99,9 @@ def delete(suffix = nil) # UsersIndex.delete '01-2014' # deletes `users_01-2014` index # def delete!(suffix = nil) - # es-ruby >= 1.0.10 handles Elasticsearch::Transport::Transport::Errors::NotFound + # es-ruby >= 1.0.10 handles Elastic::Transport::Transport::Errors::NotFound # by itself, so it is raised here - delete(suffix) or raise Elasticsearch::Transport::Transport::Errors::NotFound + delete(suffix) or raise Elastic::Transport::Transport::Errors::NotFound end # Deletes and recreates index. Supports suffixes. diff --git a/lib/chewy/index/aliases.rb b/lib/chewy/index/aliases.rb index cfcf74732..9b4a2caef 100644 --- a/lib/chewy/index/aliases.rb +++ b/lib/chewy/index/aliases.rb @@ -22,7 +22,7 @@ def aliases def empty_if_not_found yield - rescue Elasticsearch::Transport::Transport::Errors::NotFound + rescue Elastic::Transport::Transport::Errors::NotFound [] end end diff --git a/lib/chewy/index/syncer.rb b/lib/chewy/index/syncer.rb index b8365bb50..47408c9af 100644 --- a/lib/chewy/index/syncer.rb +++ b/lib/chewy/index/syncer.rb @@ -213,7 +213,7 @@ def outdated_sync_field_type @outdated_sync_field_type = mappings .fetch('properties', {}) .fetch(@index.outdated_sync_field.to_s, {})['type'] - rescue Elasticsearch::Transport::Transport::Errors::NotFound + rescue Elastic::Transport::Transport::Errors::NotFound nil end end diff --git a/lib/chewy/search/request.rb b/lib/chewy/search/request.rb index f3b1031b4..ffb93cdb0 100644 --- a/lib/chewy/search/request.rb +++ b/lib/chewy/search/request.rb @@ -854,7 +854,7 @@ def count else Chewy.client.count(only(WHERE_STORAGES).render)['count'] end - rescue Elasticsearch::Transport::Transport::Errors::NotFound + rescue Elastic::Transport::Transport::Errors::NotFound 0 end @@ -1035,7 +1035,7 @@ def perform(additional = {}) request_body = render.merge(additional) ActiveSupport::Notifications.instrument 'search_query.chewy', notification_payload(request: request_body) do Chewy.client.search(request_body) - rescue Elasticsearch::Transport::Transport::Errors::NotFound + rescue Elastic::Transport::Transport::Errors::NotFound {} end end diff --git a/spec/chewy/index/actions_spec.rb b/spec/chewy/index/actions_spec.rb index 4a0d69893..1f2456475 100644 --- a/spec/chewy/index/actions_spec.rb +++ b/spec/chewy/index/actions_spec.rb @@ -78,12 +78,12 @@ specify do expect do DummiesIndex.create! - end.to raise_error(Elasticsearch::Transport::Transport::Errors::BadRequest).with_message(/already exists.*dummies/) + end.to raise_error(Elastic::Transport::Transport::Errors::BadRequest).with_message(/already exists.*dummies/) end specify do expect do DummiesIndex.create!('2013') - end.to raise_error(Elasticsearch::Transport::Transport::Errors::BadRequest).with_message(/Invalid alias name \[dummies\]/) + end.to raise_error(Elastic::Transport::Transport::Errors::BadRequest).with_message(/Invalid alias name \[dummies\]/) end end @@ -100,7 +100,7 @@ specify do expect do DummiesIndex.create!('2013') - end.to raise_error(Elasticsearch::Transport::Transport::Errors::BadRequest).with_message(/already exists.*dummies_2013/) + end.to raise_error(Elastic::Transport::Transport::Errors::BadRequest).with_message(/already exists.*dummies_2013/) end specify { expect(DummiesIndex.create!('2014')['acknowledged']).to eq(true) } @@ -190,11 +190,11 @@ end describe '.delete!' do - specify { expect { DummiesIndex.delete! }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound) } + specify { expect { DummiesIndex.delete! }.to raise_error(Elastic::Transport::Transport::Errors::NotFound) } specify do expect do DummiesIndex.delete!('2013') - end.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound) + end.to raise_error(Elastic::Transport::Transport::Errors::NotFound) end context do @@ -768,7 +768,7 @@ .to receive(:clear_cache) .and_call_original expect { CitiesIndex.clear_cache({index: unexisted_index_name}) } - .to raise_error Elasticsearch::Transport::Transport::Errors::NotFound + .to raise_error Elastic::Transport::Transport::Errors::NotFound end end @@ -820,7 +820,7 @@ .to receive(:reindex) .and_call_original expect { CitiesIndex.reindex(source: unexisting_index, dest: dest_index_with_prefix) } - .to raise_error Elasticsearch::Transport::Transport::Errors::NotFound + .to raise_error Elastic::Transport::Transport::Errors::NotFound end end @@ -883,7 +883,7 @@ context 'index name' do specify do expect { CitiesIndex.update_mapping(unexisting_index, body_hash) } - .to raise_error Elasticsearch::Transport::Transport::Errors::NotFound + .to raise_error Elastic::Transport::Transport::Errors::NotFound end end diff --git a/spec/chewy/index/import/bulk_builder_spec.rb b/spec/chewy/index/import/bulk_builder_spec.rb index 1b2eb3a82..6ac4ab1df 100644 --- a/spec/chewy/index/import/bulk_builder_spec.rb +++ b/spec/chewy/index/import/bulk_builder_spec.rb @@ -216,7 +216,7 @@ def derived end def do_raw_index_comment(options:, data:) - CommentsIndex.client.index(options.merge(index: 'comments', type: '_doc', refresh: true, body: data)) + CommentsIndex.client.index(options.merge(index: 'comments', refresh: true, body: data)) end def raw_index_comment(comment) diff --git a/spec/chewy/index/import_spec.rb b/spec/chewy/index/import_spec.rb index 16593113a..60482a9fc 100644 --- a/spec/chewy/index/import_spec.rb +++ b/spec/chewy/index/import_spec.rb @@ -204,10 +204,10 @@ def subscribe_notification end end - let(:mapper_parsing_exception) do + let(:document_parsing_exception) do { - 'type' => 'mapper_parsing_exception', - 'reason' => 'object mapping for [name] tried to parse field [name] as object, but found a concrete value' + 'type' => 'document_parsing_exception', + 'reason' => '[1:9] object mapping for [name] tried to parse field [name] as object, but found a concrete value' } end @@ -215,7 +215,7 @@ def subscribe_notification payload = subscribe_notification import dummy_cities, batch_size: 2 expect(payload).to eq(index: CitiesIndex, - errors: {index: {mapper_parsing_exception => %w[1 2 3]}}, + errors: {index: {document_parsing_exception => %w[1 2 3]}}, import: {index: 3}) end end @@ -270,8 +270,8 @@ def subscribe_notification expect(payload).to eq( errors: { index: {{ - 'type' => 'mapper_parsing_exception', - 'reason' => 'object mapping for [object] tried to parse field [object] as object, but found a concrete value' + 'type' => 'document_parsing_exception', + 'reason' => '[1:27] object mapping for [object] tried to parse field [object] as object, but found a concrete value' } => %w[2 4]} }, import: {index: 6}, @@ -293,8 +293,8 @@ def subscribe_notification expect(payload).to eq( errors: { index: {{ - 'type' => 'mapper_parsing_exception', - 'reason' => 'object mapping for [object] tried to parse field [object] as object, but found a concrete value' + 'type' => 'document_parsing_exception', + 'reason' => '[1:27] object mapping for [object] tried to parse field [object] as object, but found a concrete value' } => %w[2 4]} }, import: {index: 6}, @@ -319,8 +319,8 @@ def subscribe_notification expect(payload).to eq( errors: { index: {{ - 'type' => 'mapper_parsing_exception', - 'reason' => 'object mapping for [object] tried to parse field [object] as object, but found a concrete value' + 'type' => 'document_parsing_exception', + 'reason' => '[1:27] object mapping for [object] tried to parse field [object] as object, but found a concrete value' } => %w[2 4]} }, import: {index: 6}, @@ -383,8 +383,8 @@ def subscribe_notification # Full match doesn't work here. expect(payload[:errors][:update].keys).to match([ - hash_including('type' => 'document_missing_exception', 'reason' => '[_doc][1]: document missing'), - hash_including('type' => 'document_missing_exception', 'reason' => '[_doc][3]: document missing') + hash_including('type' => 'document_missing_exception', 'reason' => '[1]: document missing'), + hash_including('type' => 'document_missing_exception', 'reason' => '[3]: document missing') ]) expect(payload[:errors][:update].values).to eq([['1'], ['3']]) expect(imported_cities).to match_array([ @@ -431,8 +431,8 @@ def subscribe_notification expect(payload).to eq( errors: { update: {{ - 'type' => 'mapper_parsing_exception', - 'reason' => 'object mapping for [object] tried to parse field [object] as object, but found a concrete value' + 'type' => 'document_parsing_exception', + 'reason' => '[1:26] object mapping for [object] tried to parse field [object] as object, but found a concrete value' } => %w[2 4]} }, import: {index: 6}, diff --git a/spec/chewy/index/specification_spec.rb b/spec/chewy/index/specification_spec.rb index 68a34b8d5..92457301d 100644 --- a/spec/chewy/index/specification_spec.rb +++ b/spec/chewy/index/specification_spec.rb @@ -46,7 +46,6 @@ specify do expect { specification1.lock! }.to change { Chewy::Stash::Specification.all.hits }.from([]).to([{ '_index' => 'chewy_specifications', - '_type' => '_doc', '_id' => 'places', '_score' => 1.0, '_source' => {'specification' => Base64.encode64({ @@ -62,7 +61,6 @@ specify do expect { specification5.lock! }.to change { Chewy::Stash::Specification.all.hits }.to([{ '_index' => 'chewy_specifications', - '_type' => '_doc', '_id' => 'places', '_score' => 1.0, '_source' => {'specification' => Base64.encode64({ @@ -71,7 +69,6 @@ }.to_json)} }, { '_index' => 'chewy_specifications', - '_type' => '_doc', '_id' => 'namespace/cities', '_score' => 1.0, '_source' => {'specification' => Base64.encode64({ diff --git a/spec/chewy/runtime_spec.rb b/spec/chewy/runtime_spec.rb index edc85231b..e8cc457d3 100644 --- a/spec/chewy/runtime_spec.rb +++ b/spec/chewy/runtime_spec.rb @@ -3,7 +3,7 @@ describe Chewy::Runtime do describe '.version' do specify { expect(described_class.version).to be_a(described_class::Version) } - specify { expect(described_class.version).to be >= '7.0' } - specify { expect(described_class.version).to be < '8.0' } + specify { expect(described_class.version).to be >= '8.0' } + specify { expect(described_class.version).to be < '9.0' } end end diff --git a/spec/chewy/search/response_spec.rb b/spec/chewy/search/response_spec.rb index 7b291288d..2e8cd29d8 100644 --- a/spec/chewy/search/response_spec.rb +++ b/spec/chewy/search/response_spec.rb @@ -39,7 +39,7 @@ specify { expect(subject.hits).to all be_a(Hash) } specify do expect(subject.hits.flat_map(&:keys).uniq) - .to match_array(%w[_id _index _type _score _source sort]) + .to match_array(%w[_id _index _score _source sort]) end context do diff --git a/spec/chewy_spec.rb b/spec/chewy_spec.rb index 3023b71e4..72099c035 100644 --- a/spec/chewy_spec.rb +++ b/spec/chewy_spec.rb @@ -108,7 +108,7 @@ expect(CitiesIndex.exists?).to eq true expect(PlacesIndex.exists?).to eq true - expect { Chewy.create_indices! }.to raise_error(Elasticsearch::Transport::Transport::Errors::BadRequest) + expect { Chewy.create_indices! }.to raise_error(Elastic::Transport::Transport::Errors::BadRequest) end end end From d189dded336b066643f85a09a592e2721943e14b Mon Sep 17 00:00:00 2001 From: Danil Nurgaliev Date: Mon, 18 Dec 2023 09:58:16 +0300 Subject: [PATCH 2/3] Fix termnite after changes --- lib/chewy/search/request.rb | 4 ++-- lib/chewy/search/response.rb | 7 +++++++ lib/chewy/search/scrolling.rb | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/chewy/search/request.rb b/lib/chewy/search/request.rb index ffb93cdb0..27d5cdefe 100644 --- a/lib/chewy/search/request.rb +++ b/lib/chewy/search/request.rb @@ -46,7 +46,7 @@ class Request delegate :hits, :wrappers, :objects, :records, :documents, :object_hash, :record_hash, :document_hash, - :total, :max_score, :took, :timed_out?, to: :response + :total, :max_score, :took, :timed_out?, :terminated_early?, to: :response delegate :each, :size, :to_a, :[], to: :wrappers alias_method :to_ary, :to_a alias_method :total_count, :total @@ -891,7 +891,7 @@ def exists? def first(limit = UNDEFINED) request_limit = limit == UNDEFINED ? 1 : limit - if performed? && (request_limit <= size || size == total) + if performed? && (terminated_early? || request_limit <= size || size == total) limit == UNDEFINED ? wrappers.first : wrappers.first(limit) else result = except(EXTRA_STORAGES).limit(request_limit).to_a diff --git a/lib/chewy/search/response.rb b/lib/chewy/search/response.rb index 0a5becb24..5409a113d 100644 --- a/lib/chewy/search/response.rb +++ b/lib/chewy/search/response.rb @@ -47,6 +47,13 @@ def timed_out? @timed_out ||= @body['timed_out'] end + # Has the request been terminated early? + # + # @return [true, false] + def terminated_early? + @terminated_early ||= @body['terminated_early'] + end + # The `suggest` response part. Returns empty hash if suggests # were not requested. # diff --git a/lib/chewy/search/scrolling.rb b/lib/chewy/search/scrolling.rb index 6074b3252..f0c738374 100644 --- a/lib/chewy/search/scrolling.rb +++ b/lib/chewy/search/scrolling.rb @@ -39,7 +39,8 @@ def scroll_batches(batch_size: Request::DEFAULT_BATCH_SIZE, scroll: Request::DEF hits = hits.first(last_batch_size) if last_batch_size != 0 && fetched >= total yield(hits) if hits.present? scroll_id = result['_scroll_id'] - break if fetched >= total + + break if result['terminated_early'] || fetched >= total result = perform_scroll(scroll: scroll, scroll_id: scroll_id) end From 5a1c4d540b3b3047b7f2fd104c107ff95f7cf667 Mon Sep 17 00:00:00 2001 From: Danil Nurgaliev Date: Mon, 18 Dec 2023 10:03:55 +0300 Subject: [PATCH 3/3] fixup es --- .github/workflows/ruby.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 979dadf97..04c9dd65e 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -29,11 +29,11 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true - - name: Run Elasticsearch - uses: elastic/elastic-github-actions/elasticsearch@9de0f78f306e4ebc0838f057e6b754364685e759 - with: - stack-version: 7.10.1 - port: 9250 + - name: Start containers + run: | + docker compose up elasticsearch_test -d + sleep 10 + - name: Tests run: bundle exec rspec