Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ElasticSearch v. 8 upgrade #962

Merged
merged 9 commits into from
Aug 30, 2024
Merged
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
10 changes: 5 additions & 5 deletions .github/workflows/ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use services for that? The same way we start redis.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barthez the containers created this way is quite hard to configure (yeah, we have custom configs :D ) , given that the same containers needed for local development as well 🤷 so I find it quite convenient use docker compose based ES.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, it's possible, I don't remember why we didn't use them from the start.

run: |
docker compose up elasticsearch_test -d
sleep 15

- name: Tests
run: bundle exec rspec

Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@

### Bugs Fixed

## 8.0.0-beta (2024-08-27)

### New Features

* [#962](https://github.com/toptal/chewy/pull/962): ElasticSearch v.8 support added

* `delete_all_enabled` setting introduced to align Chewy.massacre with wildcard indices deletion disabled in ES 8 by default

### Changes

### Bugs Fixed

## 7.6.0 (2024-05-03)

### Changes
Expand Down
34 changes: 33 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Chewy is compatible with MRI 3.0-3.2¹.

| Chewy version | Elasticsearch version |
| ------------- | ---------------------------------- |
| 8.0.0 | 8.x |
| 7.2.x | 7.x |
| 7.1.x | 7.x |
| 7.0.x | 6.8, 7.x |
Expand Down Expand Up @@ -97,7 +98,36 @@ development:
Make sure you have Elasticsearch up and running. You can [install](https://www.elastic.co/guide/en/elasticsearch/reference/current/install-elasticsearch.html) it locally, but the easiest way is to use [Docker](https://www.docker.com/get-started):

```shell
$ docker run --rm --name elasticsearch -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" elasticsearch:7.11.1
$ docker run --rm --name elasticsearch -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" -e "xpack.security.enabled=false" elasticsearch:8.15.0
```

### Security

Please note that starting from version 8 ElasticSearch has security features enabled by default.
Docker command above has it disabled for local testing convenience. If you want to enable it, omit
`"xpack.security.enabled=false"` part from Docker command, and run these command after starting container (container name `es8` assumed):

Reset password for `elastic` user:
```
docker container exec es8 '/usr/share/elasticsearch/bin/elasticsearch-reset-password' -u elastic
```

Extract CA certificate generated by ElasticSearch on first run:
```
docker container cp es8:/usr/share/elasticsearch/config/certs/http_ca.crt tmp/
```

And then add them to settings:

```yaml
# config/chewy.yml
development:
host: 'localhost:9200'
user: 'elastic'
password: 'SomeLongPassword'
transport_options:
ssl:
ca_file: './tmp/http_ca.crt'
```

### Index
Expand Down Expand Up @@ -941,6 +971,8 @@ Controller actions are wrapped with the configurable value of `Chewy.request_str

It is also a good idea to set up the `:bypass` strategy inside your test suite and import objects manually only when needed, and use `Chewy.massacre` when needed to flush test ES indices before every example. This will allow you to minimize unnecessary ES requests and reduce overhead.

Deprecation note: since version 8 wildcard removing of indices is disabled by default. You can enable it for a cluster with setting `action.destructive_requires_name` to false.

```ruby
RSpec.configure do |config|
config.before(:suite) do
Expand Down
5 changes: 3 additions & 2 deletions chewy.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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.14.0', '< 8'
spec.add_dependency 'activesupport', '>= 6.1'
spec.add_dependency 'elasticsearch', '>= 8.14', '< 9.0'
spec.add_dependency 'elasticsearch-dsl'
spec.metadata['rubygems_mfa_required'] = 'true'
end
15 changes: 15 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
version: "3.4"
services:
elasticsearch_test:
image: "elasticsearch:8.15.0"
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
ports:
- "127.0.0.1:9250:9200"
ulimits:
nofile:
soft: 65536
hard: 65536
4 changes: 4 additions & 0 deletions lib/chewy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ def wait_for_status
# Be careful, if current prefix is blank, this will destroy all the indexes.
#
def massacre
unless Chewy.settings[:delete_all_enabled]
raise FeatureDisabled, 'Feature disabled by default in ES 8. You can enable it in the cluster and set `delete_all_enabled` option in settings'
end

Chewy.client.indices.delete(index: [Chewy.configuration[:prefix], '*'].reject(&:blank?).join('_'))
Chewy.wait_for_status
end
Expand Down
4 changes: 2 additions & 2 deletions lib/chewy/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ def initialize
end

def transport_logger=(logger)
Chewy.client.transport.transport.logger = logger
Chewy.client.transport.logger = logger
@transport_logger = logger
end

def transport_tracer=(tracer)
Chewy.client.transport.transport.tracer = tracer
Chewy.client.transport.tracer = tracer
@transport_tracer = tracer
end

Expand Down
3 changes: 3 additions & 0 deletions lib/chewy/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,7 @@ def initialize(join_field_type, join_field_name, relations)

class ImportScopeCleanupError < Error
end

class FeatureDisabled < Error
end
end
10 changes: 5 additions & 5 deletions lib/chewy/index/actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that we have very small difference between 7.x and 8.x, can we make it work with both version? For instance as a possibility, define these constants like

if es_8?
  ELASTIC_ERROR_NOT_BAD_REQUEST = Elastic::Transport::Transport::Errors::BadRequest end
else 
  ELASTIC_ERROR_NOT_BAD_REQUEST = Elasticsearch::Transport::Transport::Errors::BadRequest 
end

and use it

 def create(*args, **kwargs)
          create!(*args, **kwargs)
        rescue ELASTIC_ERROR_NOT_BAD_REQUEST

Or even make this prefix configurable

if es_8?
  ELASTIC_LIB_CONST_PREFIX = Elastic
else 
  ELASTIC_LIB_CONST_PREFIX = Elasticsearch
end

and then ELASTIC_LIB_CONST_PREFIX::Transport::Transport::Errors::BadRequest, not sure if it's going to work :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, it's not likely that we need it: #962 (comment)

false
end

Expand Down Expand Up @@ -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

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion lib/chewy/index/aliases.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/chewy/index/syncer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/chewy/minitest/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def index_everything!
teardown do
# always destroy indexes between tests
# Prevent croll pollution of test cases due to indexing
Chewy.massacre
drop_indices
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/chewy/search/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions lib/chewy/search/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
3 changes: 2 additions & 1 deletion lib/chewy/search/scrolling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/chewy/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

specify do
expect { subject.transport_logger = logger }
.to change { Chewy.client.transport.transport.logger }.to(logger)
.to change { Chewy.client.transport.logger }.to(logger)
end
specify do
expect { subject.transport_logger = logger }
Expand All @@ -40,7 +40,7 @@

specify do
expect { subject.transport_tracer = tracer }
.to change { Chewy.client.transport.transport.tracer }.to(tracer)
.to change { Chewy.client.transport.tracer }.to(tracer)
end
specify do
expect { subject.transport_tracer = tracer }
Expand Down
2 changes: 1 addition & 1 deletion spec/chewy/elastic_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let!(:filter_previous_value) { Chewy.before_es_request_filter }

before do
Chewy.massacre
drop_indices
stub_index(:products) do
field :id, type: :integer
end
Expand Down
2 changes: 1 addition & 1 deletion spec/chewy/fields/time_fields_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe 'Time fields' do
before { Chewy.massacre }
before { drop_indices }

before do
stub_index(:posts) do
Expand Down
18 changes: 9 additions & 9 deletions spec/chewy/index/actions_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe Chewy::Index::Actions do
before { Chewy.massacre }
before { drop_indices }

before do
stub_index :dummies
Expand Down Expand Up @@ -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

Expand All @@ -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) }

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/chewy/index/aliases_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe Chewy::Index::Aliases do
before { Chewy.massacre }
before { drop_indices }

before { stub_index :dummies }

Expand Down
Loading